diff --git a/backend/alembic/versions/20260920_0730_add_attribution_columns.py b/backend/alembic/versions/20260920_0730_add_attribution_columns.py new file mode 100644 index 0000000..e23b150 --- /dev/null +++ b/backend/alembic/versions/20260920_0730_add_attribution_columns.py @@ -0,0 +1,67 @@ +"""add attribution columns to asset + +M10. Eight columns recording *whose work* an asset is, as opposed to `source`, which +records how the file arrived. The two are conflated constantly and answer different +questions; see docs/m10-attribution.md for the decisions behind the field set. + +Two of the columns look wrong at a glance and are not: + +- `published_date` is a string, not a DateTime. Publication dates are routinely partial + ("1994", "March 2019") and a DateTime cannot hold either without inventing a precision + that then reads as real. ISO 8601 partial dates compare correctly as plain strings, so + the filters that use it need no parsing. It is indexed because the date range filter + orders by it. +- `license` shadows a Python builtin name only in the interactive interpreter's `site` + namespace, never as a model attribute or a SQL identifier. SQLite has no reserved word + here and SQLAlchemy quotes identifiers regardless. + +Adding columns and an index are both things SQLite's ALTER supports directly, so batch +mode does not recreate `asset` here and the PRAGMA foreign_keys dance that +`7d4b9c1a6f28` needed does not apply — nothing drops the table, so nothing can trip over +a row referencing it. + +The `asset_fts` column that makes these searchable is deliberately *not* here: it has to +drop and rebuild a virtual table, which is the risky half, and it gets its own revision +so a failure there does not strand these columns. + +Revision ID: 3f7a21c9d4e5 +Revises: 7d4b9c1a6f28 +Create Date: 2026-09-20 07:30:00.000000+00:00 +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +import sqlmodel + + +revision: str = '3f7a21c9d4e5' +down_revision: Union[str, None] = '7d4b9c1a6f28' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + with op.batch_alter_table('asset', schema=None) as batch_op: + batch_op.add_column(sa.Column('source_url', sqlmodel.sql.sqltypes.AutoString(), nullable=True)) + batch_op.add_column(sa.Column('creator', sqlmodel.sql.sqltypes.AutoString(), nullable=True)) + batch_op.add_column(sa.Column('publisher', sqlmodel.sql.sqltypes.AutoString(), nullable=True)) + batch_op.add_column(sa.Column('source_title', sqlmodel.sql.sqltypes.AutoString(), nullable=True)) + batch_op.add_column(sa.Column('published_date', sqlmodel.sql.sqltypes.AutoString(), nullable=True)) + batch_op.add_column(sa.Column('retrieved_at', sa.DateTime(), nullable=True)) + batch_op.add_column(sa.Column('license', sqlmodel.sql.sqltypes.AutoString(), nullable=True)) + batch_op.add_column(sa.Column('credit_line', sqlmodel.sql.sqltypes.AutoString(), nullable=True)) + batch_op.create_index('ix_asset_published_date', ['published_date'], unique=False) + + +def downgrade() -> None: + with op.batch_alter_table('asset', schema=None) as batch_op: + batch_op.drop_index('ix_asset_published_date') + batch_op.drop_column('credit_line') + batch_op.drop_column('license') + batch_op.drop_column('retrieved_at') + batch_op.drop_column('published_date') + batch_op.drop_column('source_title') + batch_op.drop_column('publisher') + batch_op.drop_column('creator') + batch_op.drop_column('source_url') diff --git a/backend/alembic/versions/20260920_0731_rebuild_asset_fts_with_attribution.py b/backend/alembic/versions/20260920_0731_rebuild_asset_fts_with_attribution.py new file mode 100644 index 0000000..8af3461 --- /dev/null +++ b/backend/alembic/versions/20260920_0731_rebuild_asset_fts_with_attribution.py @@ -0,0 +1,130 @@ +"""rebuild asset_fts with an attribution column + +M10. Finding an asset by its publisher is half the point of recording one, so the +attribution fields have to reach the keyword index. + +**SQLite FTS5 has no ALTER TABLE ADD COLUMN.** The only way to add `attribution_text` is +to drop the virtual table and create it again — and because `asset_fts` stores its own +copy of the text rather than using external-content mode (see the reasoning at the top of +app/search/fts.py), dropping it destroys the index for every existing asset. So this +migration repopulates, and that repopulate is not optional decoration: a recreate without +it passes any test that only compares columns, and leaves a populated library with +keyword search silently returning nothing until each asset happens to be edited again. + +Split from `3f7a21c9d4e5` precisely because this half can fail and that half cannot. If +this revision dies partway, the columns are still committed and this is re-runnable; +folded together, a failure here would strand them mid-migration. + +The repopulated text is composed to match `app/search/fts.py::index_asset`, so the first +ordinary edit of an asset does not silently rewrite its index entry into something +different. Tag order differs from `tags_text_for`'s (group_concat does not promise one) +and that is immaterial — FTS5 tokenises, so order never reaches the index. + +`published_date` and `retrieved_at` are deliberately left out of the text: both are +served exactly by the date-range filter on `/api/assets`, and a bare year in a free-text +index mostly collides with titles rather than helping. + +Revision ID: 9c2e08b4a1f7 +Revises: 3f7a21c9d4e5 +Create Date: 2026-09-20 07:31:00.000000+00:00 +""" +from typing import Sequence, Union + +from alembic import op + +revision: str = '9c2e08b4a1f7' +down_revision: Union[str, None] = '3f7a21c9d4e5' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +# This migration carries its own literal copy of the DDL rather than importing the +# constants in app/search/fts.py, per the convention that migration set out: a migration +# has to describe the schema as it was when it was written, and one that imports live +# code silently changes meaning when that code changes. test_migrations.py asserts the +# two copies still agree, which is what stops them drifting unnoticed. +ASSET_FTS_DDL = """ +CREATE VIRTUAL TABLE asset_fts USING fts5( + asset_id UNINDEXED, + user_id UNINDEXED, + name, + description, + summary, + tags_text, + attribution_text, + tokenize='porter unicode61' +) +""" + +REPOPULATE = """ +INSERT INTO asset_fts ( + asset_id, user_id, name, description, summary, tags_text, attribution_text +) +SELECT + a.id, + a.user_id, + COALESCE(a.name, ''), + COALESCE(a.description, ''), + COALESCE(a.summary, ''), + COALESCE( + (SELECT group_concat(t.name, ' ') + FROM assettag at + JOIN tag t ON t.id = at.tag_id + WHERE at.asset_id = a.id), + '' + ), + TRIM( + COALESCE(a.creator, '') || ' ' || + COALESCE(a.publisher, '') || ' ' || + COALESCE(a.source_title, '') || ' ' || + COALESCE(a.license, '') || ' ' || + COALESCE(a.credit_line, '') || ' ' || + COALESCE(a.source_url, '') + ) +FROM asset a +""" + +# The pre-M10 shape, for downgrade. Same reasoning: literal, not imported. +ASSET_FTS_DDL_WITHOUT_ATTRIBUTION = """ +CREATE VIRTUAL TABLE asset_fts USING fts5( + asset_id UNINDEXED, + user_id UNINDEXED, + name, + description, + summary, + tags_text, + tokenize='porter unicode61' +) +""" + +REPOPULATE_WITHOUT_ATTRIBUTION = """ +INSERT INTO asset_fts (asset_id, user_id, name, description, summary, tags_text) +SELECT + a.id, + a.user_id, + COALESCE(a.name, ''), + COALESCE(a.description, ''), + COALESCE(a.summary, ''), + COALESCE( + (SELECT group_concat(t.name, ' ') + FROM assettag at + JOIN tag t ON t.id = at.tag_id + WHERE at.asset_id = a.id), + '' + ) +FROM asset a +""" + + +def upgrade() -> None: + op.execute("DROP TABLE IF EXISTS asset_fts") + op.execute(ASSET_FTS_DDL) + op.execute(REPOPULATE) + + +def downgrade() -> None: + # Symmetric, and repopulating for the same reason: a downgrade that left the index + # empty would be a silent data-shaped loss, not a schema change. + op.execute("DROP TABLE IF EXISTS asset_fts") + op.execute(ASSET_FTS_DDL_WITHOUT_ATTRIBUTION) + op.execute(REPOPULATE_WITHOUT_ATTRIBUTION) diff --git a/backend/app/attribution.py b/backend/app/attribution.py new file mode 100644 index 0000000..934bc2c --- /dev/null +++ b/backend/app/attribution.py @@ -0,0 +1,192 @@ +"""Whose work an asset is, and how that resolves through a clip to its parent. + +`Asset.source` says how a file arrived — uploaded, generated, cut from something else. +This module is about the other question entirely: who made it, who published it, what it +was part of, and what may be done with it. Recording that at ingest costs a form field; +reconstructing it later means opening every file by hand, and for anything gathered from +the open web the answer is frequently gone. The decisions behind the field set, and the +alternatives rejected on the way, are in docs/m10-attribution.md. + +Everything here is pure: no session, no I/O. That is what lets the composition and +inheritance rules be tested exhaustively without a database, and it is why the callers +that *do* touch a session (services/assets.py) pass the parent in rather than having this +module go and find one. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from datetime import datetime +from typing import Any, Optional, Protocol + +# Every attribution field, in one place, so the harvester, the suggestion path, the +# schemas and the index builder cannot drift apart. Adding a ninth field means adding it +# here and following the type errors. +ATTRIBUTION_FIELDS: tuple[str, ...] = ( + "source_url", + "creator", + "publisher", + "source_title", + "published_date", + "retrieved_at", + "license", + "credit_line", +) + +# The subset that reaches the keyword index — the who and the where. +# +# `published_date` and `retrieved_at` are left out on purpose: both are answered exactly +# by the date-range filters on /api/assets, and a bare year in a free-text index mostly +# collides with titles instead of helping. The migration that builds `asset_fts` carries +# its own literal copy of this list; test_attribution.py asserts they still agree. +ATTRIBUTION_TEXT_FIELDS: tuple[str, ...] = ( + "creator", + "publisher", + "source_title", + "license", + "credit_line", + "source_url", +) + +# What a composed credit line strings together, in reading order. +_CREDIT_ORDER: tuple[str, ...] = ( + "creator", + "source_title", + "publisher", + "published_date", + "license", +) + +_CREDIT_SEPARATOR = " — " + + +class HasAttribution(Protocol): + """Structural type for the parts of `Asset` this module reads. + + A Protocol rather than importing Asset: it keeps this module free of the model layer + (so the tests can drive it with a two-field stub) and documents exactly which columns + the attribution rules depend on. + """ + + source_url: Optional[str] + creator: Optional[str] + publisher: Optional[str] + source_title: Optional[str] + published_date: Optional[str] + retrieved_at: Optional[datetime] + license: Optional[str] + credit_line: Optional[str] + + +@dataclass(frozen=True) +class ResolvedAttribution: + """One asset's effective attribution, after inheritance.""" + + source_url: Optional[str] = None + creator: Optional[str] = None + publisher: Optional[str] = None + source_title: Optional[str] = None + published_date: Optional[str] = None + retrieved_at: Optional[datetime] = None + license: Optional[str] = None + credit_line: Optional[str] = None + # Which of the above came from the parent rather than from the asset itself. The API + # hands this to the UI so an inherited value can be shown as inherited, rather than + # looking like something typed on the clip and then quietly diverging from it. + inherited: tuple[str, ...] = () + + @property + def credit(self) -> str: + """The line to display: the override if there is one, else the composition.""" + if not _is_blank(self.credit_line): + return str(self.credit_line).strip() + return compose_credit(self) + + @property + def is_empty(self) -> bool: + """True when nothing is recorded at all — what the `unattributed` filter means.""" + return all(_is_blank(getattr(self, name)) for name in ATTRIBUTION_FIELDS) + + +def _is_blank(value: Any) -> bool: + """Empty for attribution purposes: None, or a string that is only whitespace. + + A datetime is never blank. Written as one helper because "did the user actually put + something here" is asked by inheritance, by composition and by the unattributed + filter, and three subtly different answers would be three subtly different bugs. + """ + if value is None: + return True + if isinstance(value, str): + return not value.strip() + return False + + +def compose_credit(source: Any) -> str: + """Build a one-line citation from whatever fields are filled in. + + Deliberately mechanical: non-empty parts, in a fixed order, joined by an em dash. + A cleverer format ("Doe, J. *Panorama* (BBC, 2019)") needs rules for every + combination of missing fields and gets them wrong for the combination nobody tried. + `credit_line` exists precisely so a user who wants a specific wording can write it, + and this never has to guess on their behalf. + + Ignores `credit_line` itself — this is what a credit line is composed *from*. Use + `ResolvedAttribution.credit` to get the override-or-composition. + """ + parts = [] + for name in _CREDIT_ORDER: + value = getattr(source, name, None) + if not _is_blank(value): + parts.append(str(value).strip()) + return _CREDIT_SEPARATOR.join(parts) + + +def resolve( + asset: Any, parent: Any = None +) -> ResolvedAttribution: + """An asset's effective attribution, falling back to its parent field by field. + + A clip of a documentary has the documentary's publisher without anyone retyping it, + and correcting the documentary corrects every clip — resolution happens on read, so + there is no copy anywhere to go stale. Override is per field, not all or nothing, so + a clip can carry its own creator for one interviewee while keeping the rest. + + **One level, and that is complete rather than a simplification.** + `services/assets.py::create_clip` refuses to clip a clip, and `promote_clip` leaves + `parent_asset_id` pointing at the original, so no chain of length two can exist. Do + not add a recursive walk for a depth the write paths cannot produce. + """ + values: dict[str, Any] = {} + inherited: list[str] = [] + + for name in ATTRIBUTION_FIELDS: + own = getattr(asset, name, None) + if not _is_blank(own): + values[name] = own + continue + + from_parent = getattr(parent, name, None) if parent is not None else None + if not _is_blank(from_parent): + values[name] = from_parent + inherited.append(name) + else: + values[name] = None + + return ResolvedAttribution(**values, inherited=tuple(inherited)) + + +def index_text(resolved: Any) -> str: + """The attribution text that goes into `asset_fts`. + + Takes a resolved attribution (or a bare asset) so a clip is findable by the publisher + it inherited, not only by the fields typed on the clip itself. Keeping a clip out of + those results would make "everything from the BBC" quietly incomplete in a way the + user has no way to notice. + """ + parts = [] + for name in ATTRIBUTION_TEXT_FIELDS: + value = getattr(resolved, name, None) + if not _is_blank(value): + parts.append(str(value).strip()) + return " ".join(parts) diff --git a/backend/app/enrichment/attribute.py b/backend/app/enrichment/attribute.py new file mode 100644 index 0000000..9b4f28e --- /dev/null +++ b/backend/app/enrichment/attribute.py @@ -0,0 +1,231 @@ +"""The attribute job: propose where this came from, and write none of it. + +The only enrichment in GAM that may not write directly, and the asymmetry is deliberate. +A description the model gets wrong is a poorer search result. A *citation* the model gets +wrong credits somebody else's work to the wrong outlet, and it does so in a field that +looks finished — a blank publisher prompts a fix, a confidently wrong one does not. So +this produces `Suggestion` rows and stops, the same shape FR 9.1.4 gives autotag. + +The grounding rule is the other half of that. The model may only return a field it can +quote the text it read it from — a chyron, a byline, a title page, a watermark — and that +quote is stored on the suggestion and shown in the panel. A field it cannot ground is +omitted rather than guessed, because a plausible invention is exactly the failure this +whole design is arranged against, and a reviewer who cannot see what the model read is +not really reviewing anything. +""" + +from __future__ import annotations + +import json +import logging +import re +from typing import Any, Callable + +from sqlmodel import Session + +from app.enrichment import source +from app.ingest.embedded_metadata import normalise_partial_date +from app.models.asset import Asset +from app.providers import build_provider +from app.providers.base import ProviderError, ProviderUnavailable +from app.services import suggestions as suggestion_service +from app.usage import events as usage_events + +logger = logging.getLogger(__name__) + +Progress = Callable[..., None] + +# What the model may propose. `credit_line` is excluded because it is composed from these +# — proposing a finished sentence would put wording in front of the user that no +# component field supports. `retrieved_at` is excluded because when *you* fetched +# something is not in the content. +PROPOSABLE = ("creator", "publisher", "source_title", "published_date", "source_url", "license") + +SYSTEM_PROMPT = ( + "You identify where a piece of media came from, so it can be credited.\n\n" + "Reply with JSON only, in exactly this shape:\n" + '{"fields": [{"field": "publisher", "value": "BBC Two", ' + '"evidence": "the on-screen logo in the corner reads BBC TWO"}]}\n\n' + "Allowed values for \"field\": creator, publisher, source_title, published_date, " + "source_url, license.\n\n" + " creator — the person who made it: author, photographer, speaker, director.\n" + " publisher — the outlet, channel, studio or imprint that released it.\n" + " source_title — the programme, film, article or book this is part of.\n" + " published_date — when it was first published, as YYYY, YYYY-MM or YYYY-MM-DD. " + "Give only the precision you can actually support: a year alone is a good answer.\n" + " source_url — a web address visibly present in the material.\n" + " license — a copyright or licence statement visibly present in the material.\n\n" + "THE RULE THAT MATTERS: include a field ONLY if you can quote the specific thing in " + "the material that tells you — a caption, a chyron or lower third, a watermark, a " + "spoken introduction, a byline, a title page, a copyright notice. Put that quote in " + "\"evidence\". If you cannot point to something, leave the field out entirely.\n\n" + "Do not infer from style, subject matter, production values, or what is typical. Do " + "not guess a plausible outlet. An empty list is a correct and useful answer — a " + "wrong citation is far worse than a missing one.\n\n" + 'If you can ground nothing, reply exactly {"fields": []}.\n\n' + "Output the JSON and nothing else: no explanation, no markdown fence." +) + + +def _prompt(asset: Asset, material: source.SourceMaterial) -> str: + parts = [f"Filename: {asset.original_name or asset.name}"] + if asset.description: + parts.append(f"The owner's own note: {asset.description}") + + # What is already recorded, so the model does not spend its answer restating it — + # and so it can tell that a blank field is genuinely blank rather than withheld. + known = { + name: getattr(asset, name) + for name in PROPOSABLE + if isinstance(getattr(asset, name, None), str) and getattr(asset, name).strip() + } + if known: + parts.append( + "Already recorded, do not propose these again:\n" + + "\n".join(f" {name}: {value}" for name, value in known.items()) + ) + + if material.kind == source.FROM_TRANSCRIPT: + header = "Transcript" + if material.truncated: + header += " (the opening portion only)" + parts.append(f"{header}:\n\n{material.text}") + elif material.kind == source.FROM_DOCUMENT: + header = "Document text" + if material.truncated: + header += " (the opening portion only)" + parts.append(f"{header}:\n\n{material.text}") + elif material.kind == source.FROM_POSTER: + parts.append( + "No transcript is available; a single frame from the video is attached. " + "Look for a channel logo, a lower third, a watermark or a caption." + ) + else: + parts.append( + "The image itself is attached. Look for a watermark, a credit line, a " + "caption or a visible byline." + ) + + return "\n\n".join(parts) + + +_FENCE = re.compile(r"^```(?:json)?\s*|\s*```$", re.MULTILINE) + + +def parse_reply(text: str) -> list[dict]: + """Pull grounded field proposals out of whatever came back. + + Split from the request so it can be tested against recorded replies, which is where + the grounding rule is actually enforced: a model that returns a field with no + evidence has not followed the instruction, and this drops it rather than trusting it. + """ + cleaned = _FENCE.sub("", text or "").strip() + if not cleaned: + raise ProviderError("The provider returned an empty reply") + + try: + payload: Any = json.loads(cleaned) + except ValueError: + # Some models prepend a sentence despite being told not to. Take the outermost + # object rather than discarding an otherwise usable answer. + start, end = cleaned.find("{"), cleaned.rfind("}") + if start == -1 or end <= start: + raise ProviderError("The provider did not return usable JSON") from None + try: + payload = json.loads(cleaned[start : end + 1]) + except ValueError: + raise ProviderError("The provider did not return usable JSON") from None + + if not isinstance(payload, dict): + raise ProviderError("The provider did not return usable JSON") + + raw = payload.get("fields") + if not isinstance(raw, list): + raise ProviderError("The provider did not return usable JSON") + + found: list[dict] = [] + for entry in raw: + if not isinstance(entry, dict): + continue + + name = entry.get("field") + value = entry.get("value") + evidence = entry.get("evidence") + + if name not in PROPOSABLE: + continue + if not isinstance(value, str) or not value.strip(): + continue + + # The grounding rule, enforced rather than requested. A model that returns a + # publisher with no evidence has guessed, whatever it says in the prompt — and + # an ungrounded citation is the precise failure this job is shaped to avoid. + if not isinstance(evidence, str) or not evidence.strip(): + logger.info("Dropping ungrounded attribution proposal for %s", name) + continue + + cleaned_value = value.strip() + if name == "published_date": + # The model is asked for a partial ISO date and will sometimes answer + # "March 2019" or "2019-03-15T00:00:00". Normalised through the same reducer + # the file harvester uses, and dropped if it cannot be read at all — the + # column has a format and the API will refuse anything else. + normalised = normalise_partial_date(cleaned_value) + if not normalised: + continue + cleaned_value = normalised + + found.append( + {"field": name, "value": cleaned_value, "evidence": evidence.strip()} + ) + + return found + + +def run(session: Session, asset: Asset, progress: Progress) -> str: + """Propose attribution for one asset. Returns a detail line for the job row.""" + provider = build_provider(session, asset.user_id) + if provider is None: + raise ProviderUnavailable( + "No AI provider is configured. Add one in Settings to enable enrichment." + ) + + progress("Reading the asset", 10, "") + material = source.gather(session, asset, supports_images=provider.supports_images) + + progress("Looking for a source", 30, "") + completion = provider.complete( + _prompt(asset, material), + system=SYSTEM_PROMPT, + images=material.images, + ) + + # Recorded before the reply is parsed, so a run that comes back as unusable JSON is + # still accounted for — the tokens were spent either way. + usage_events.record_completion(session, asset.id, asset.user_id, completion) + + proposals = parse_reply(completion.text) + + # Checkpointed before the write, so a cancel during a long completion does not still + # land suggestions afterwards. + progress("Saving suggestions", 90, "") + + created = suggestion_service.propose_attribution( + session, asset, proposals=proposals, model=completion.model + ) + + if not created: + # A true and useful answer, and the one the grounding rule is meant to produce + # often: most material does not say where it came from, and saying so is better + # than filling the panel with plausible inventions. + return "Nothing it could point to" + + logger.info( + "Attributed asset %s from %s: %d suggestion(s), %d in / %d out tokens", + asset.id, + material.kind, + created, + completion.usage.input_tokens, + completion.usage.output_tokens, + ) + return f"{created} suggestion{'' if created == 1 else 's'} to review" diff --git a/backend/app/enrichment/bulk.py b/backend/app/enrichment/bulk.py index dcf395e..78ed12e 100644 --- a/backend/app/enrichment/bulk.py +++ b/backend/app/enrichment/bulk.py @@ -19,9 +19,10 @@ from sqlmodel import Session -from app.enrichment import autotag, describe, embed, extract_text, summarize +from app.enrichment import attribute, autotag, describe, embed, extract_text, summarize from app.models.asset import Asset from app.models.job import ( + KIND_ATTRIBUTE, KIND_AUTOTAG, KIND_DESCRIBE, KIND_EMBED, @@ -45,6 +46,10 @@ KIND_DESCRIBE: describe.run, KIND_SUMMARIZE: summarize.run, KIND_AUTOTAG: autotag.run, + # The action most worth having over a selection after extract_text: twenty + # screenshots grabbed from one programme in one sitting share every attribution + # field, and it writes nothing either way — every result is a suggestion. + KIND_ATTRIBUTE: attribute.run, KIND_EMBED: embed.run, # Safe to include where transcription is not: it calls nothing and bills nothing, so # the mis-click that makes transcription too expensive to offer here costs only time. diff --git a/backend/app/enrichment/harvest_attribution.py b/backend/app/enrichment/harvest_attribution.py new file mode 100644 index 0000000..f0874cd --- /dev/null +++ b/backend/app/enrichment/harvest_attribution.py @@ -0,0 +1,98 @@ +"""Re-reading embedded attribution across a whole library. + +Ingest harvests every new upload, so anything added from M10 onward takes care of +itself. This is for everything added before — which, on an instance that has been +running a while, is the entire library. Those files still carry their EXIF and ID3 and +PDF Author on disk; nothing has ever looked. + +One job walks the lot, for the same reasons `backfill.py` gives: one cancellable row, +one progress bar, one line in the activity feed rather than a wall of near-identical +ones. + +Safe to run repeatedly. It only ever fills fields that are blank, so a second run is a +no-op over everything the first one filled and over everything the user has since +corrected by hand — there is no "already harvested" flag to keep, and no way for this to +walk back over an answer. +""" + +from __future__ import annotations + +import logging +from dataclasses import dataclass +from typing import Callable + +from sqlmodel import Session, col, select + +from app.ingest import embedded_metadata +from app.ingest.probe import probe +from app.models.asset import Asset +from app.services import assets as asset_service +from app.storage import StorageError, build_storage + +logger = logging.getLogger(__name__) + +Progress = Callable[..., None] + + +@dataclass +class HarvestResult: + scanned: int + attributed: int + failed: int + + +def run(session: Session, user_id: str, progress: Progress) -> HarvestResult: + """Harvest every asset of this user's that owns a file.""" + storage = build_storage() + + # Clips are excluded by `storage_key IS NOT NULL`: a clip owns no bytes, and it + # inherits its parent's attribution on read anyway, so there is nothing here for it. + pending = list( + session.exec( + select(Asset) + .where(Asset.user_id == user_id, col(Asset.storage_key).is_not(None)) + .order_by(col(Asset.upload_date).desc()) + ).all() + ) + total = len(pending) + + scanned = attributed = failed = 0 + + for index, asset in enumerate(pending): + pct = int(index * 100 / total) if total else 100 + progress("Reading file metadata", pct, f"{index + 1} of {total} · {asset.name}") + + try: + found = _harvest_one(storage, asset) + except StorageError: + # The file went missing underneath the row. Not this job's problem to fix, + # and not a reason to stop. + failed += 1 + continue + except Exception: # noqa: BLE001 - one unreadable file must not end the run + failed += 1 + logger.warning("Could not harvest attribution for %s", asset.id, exc_info=True) + continue + + scanned += 1 + if not found: + continue + + before = {name: getattr(asset, name, None) for name in found} + asset_service.apply_embedded_attribution(session, asset, found) + session.refresh(asset) + if any(getattr(asset, name, None) != before[name] for name in found): + attributed += 1 + + return HarvestResult(scanned=scanned, attributed=attributed, failed=failed) + + +def _harvest_one(storage, asset: Asset) -> dict: + if not asset.storage_key: + return {} + + with storage.materialise(asset.storage_key) as path: + tags = probe(path).tags if asset.asset_type in ("video", "audio") else {} + return embedded_metadata.harvest( + path, asset.asset_type, asset.original_name or "", probe_tags=tags + ) diff --git a/backend/app/ingest/embedded_metadata.py b/backend/app/ingest/embedded_metadata.py new file mode 100644 index 0000000..7ae4b99 --- /dev/null +++ b/backend/app/ingest/embedded_metadata.py @@ -0,0 +1,402 @@ +"""Attribution the file already carries. + +A JPEG's EXIF `Artist`, an MP3's ID3 frames, an MP4's tag block, a PDF's `Author`, a +.docx's core properties — all written by whoever produced the file. Reading them is not +inference, which is why this writes directly while an AI proposal has to go through the +suggestion queue. The asymmetry is the point; see docs/m10-attribution.md. + +None of it is new data on the wire: `ingest/probe.py` has always run ffprobe with +`-show_format`, which returns the tag block, and GAM parsed out the duration and threw +the rest away. + +Everything here degrades rather than fails, and for the same reason the rest of the +ingest pipeline does: this runs after the row is committed, and a file with unreadable +metadata is still a perfectly good asset. + +What is deliberately *not* harvested: + +- `retrieved_at` — when *you* fetched something is not a fact the file can know. +- `name` — set from the filename at ingest and the user's to change. A container's + `title` is frequently boilerplate from an encoder, and silently renaming somebody's + asset on upload is a worse failure than leaving a field blank. +- `description` / `summary` — not attribution, and owned by the enrichment path. +""" + +from __future__ import annotations + +import logging +import re +from datetime import datetime +from pathlib import Path +from typing import Any, Mapping, Optional + +from app.ingest.filetypes import TYPE_AUDIO, TYPE_DOCUMENT, TYPE_IMAGE, TYPE_VIDEO, extension_of + +logger = logging.getLogger(__name__) + +# EXIF tag numbers, which Pillow returns as an int-keyed mapping. +_EXIF_ARTIST = 0x013B +_EXIF_COPYRIGHT = 0x8298 +_EXIF_DATETIME_ORIGINAL = 0x9003 +_EXIF_DATETIME = 0x0132 + +# IPTC records, as Pillow's IptcImagePlugin keys them: (record, dataset). +_IPTC_BYLINE = (2, 80) +_IPTC_CREDIT = (2, 110) +_IPTC_SOURCE = (2, 115) + +# Container tag names, in preference order, per attribution field. +# +# `title` is absent on purpose. In a media container it names *this file*, not a +# containing work, and it is very often an encoder's boilerplate — so it maps to neither +# `name` (see the module docstring) nor `source_title`. `album` and `show` genuinely do +# name a containing work, so they are here. Documents are the other way round and are +# handled separately below: a document's `title` is the work's own title, and there is no +# album to stand in for it. +_TAG_SOURCES: dict[str, tuple[str, ...]] = { + "creator": ("artist", "author", "album_artist", "composer", "director"), + "publisher": ("publisher", "label", "network", "studio"), + "source_title": ("album", "show"), + "license": ("copyright", "license", "rights"), + "published_date": ("date", "originaldate", "year", "creation_time"), + "source_url": ("purl", "url", "wxxx", "comment"), +} + + +def harvest( + path: Path, + asset_type: str, + original_name: str = "", + probe_tags: Optional[Mapping[str, str]] = None, +) -> dict[str, Any]: + """Attribution fields read out of this file. Never raises. + + Returns only fields it actually found — the caller decides what to do with them, and + `services/assets.py` writes only into fields that are still empty, so a harvest can + never clobber something a person typed. + """ + try: + if asset_type in (TYPE_VIDEO, TYPE_AUDIO): + return _from_container_tags(probe_tags or {}) + if asset_type == TYPE_IMAGE: + return _from_image(path) + if asset_type == TYPE_DOCUMENT: + return _from_document(path, original_name or path.name) + except Exception as exc: # noqa: BLE001 - metadata must never fail an upload + logger.warning("Could not read embedded metadata from %s: %s", path.name, exc) + return {} + + +# ─── video and audio ───────────────────────────────────────────────────────── + + +def _from_container_tags(tags: Mapping[str, str]) -> dict[str, Any]: + """Map an ffprobe tag block onto attribution fields. + + Only the keys in `_TAG_SOURCES` are read. That allowlist is deliberate: a container + tag block is mostly technical noise (`encoder`, `handler_name`, `major_brand`, + `compatible_brands`) and a mapping that took whatever it recognised would file + "Lavf60.16.100" as somebody's creator. + """ + found: dict[str, Any] = {} + + for field_name, candidates in _TAG_SOURCES.items(): + for key in candidates: + value = (tags.get(key) or "").strip() + if not value: + continue + + if field_name == "published_date": + normalised = normalise_partial_date(value) + if normalised: + found[field_name] = normalised + break + continue + + if field_name == "source_url": + # `comment` is in the candidate list because downloaders routinely put + # the source URL there — and also because they put everything else + # there. Only take it when it is actually a URL. + if _looks_like_url(value): + found[field_name] = value + break + continue + + found[field_name] = value + break + + return found + + +# ─── images ────────────────────────────────────────────────────────────────── + + +def _from_image(path: Path) -> dict[str, Any]: + from PIL import Image + + found: dict[str, Any] = {} + with Image.open(path) as image: + exif = image.getexif() + if exif: + artist = _clean(exif.get(_EXIF_ARTIST)) + if artist: + found["creator"] = artist + + rights = _clean(exif.get(_EXIF_COPYRIGHT)) + if rights: + found["license"] = rights + + # DateTimeOriginal is when the shutter fired; DateTime is when the file was + # last written, which an edit updates. Prefer the former. + # + # DateTimeOriginal lives in the Exif sub-IFD (0x8769), not IFD0, so a + # top-level lookup alone finds it in hand-built files and misses it in every + # real photograph — which is the wrong way round for a test to pass. + sub_ifd = {} + try: + sub_ifd = exif.get_ifd(0x8769) or {} + except Exception: # noqa: BLE001 - a malformed sub-IFD is not worth failing over + sub_ifd = {} + + shot = ( + _clean(sub_ifd.get(_EXIF_DATETIME_ORIGINAL)) + or _clean(exif.get(_EXIF_DATETIME_ORIGINAL)) + or _clean(exif.get(_EXIF_DATETIME)) + ) + taken = normalise_partial_date(shot) if shot else None + if taken: + found["published_date"] = taken + + found.update(_from_iptc(image)) + + return found + + +def _from_iptc(image: Any) -> dict[str, Any]: + """IPTC, which is where a press photo's real credit lives. + + EXIF `Artist` is usually the camera owner; a wire photo carries By-line, Credit and + Source instead, and those are the fields a picture desk actually fills in. They win + over EXIF for that reason. + """ + from PIL import IptcImagePlugin + + try: + info = IptcImagePlugin.getiptcinfo(image) + except Exception: # noqa: BLE001 - malformed IPTC is common and not our problem + return {} + if not info: + return {} + + found: dict[str, Any] = {} + byline = _clean(_decode_iptc(info.get(_IPTC_BYLINE))) + if byline: + found["creator"] = byline + + source = _clean(_decode_iptc(info.get(_IPTC_SOURCE))) + if source: + found["publisher"] = source + + credit = _clean(_decode_iptc(info.get(_IPTC_CREDIT))) + if credit: + found["credit_line"] = credit + + return found + + +def _decode_iptc(value: Any) -> Optional[str]: + """IPTC values are bytes, and repeat-able fields come back as a list of them.""" + if isinstance(value, (list, tuple)): + value = value[0] if value else None + if isinstance(value, bytes): + return value.decode("utf-8", errors="replace") + return value if isinstance(value, str) else None + + +# ─── documents ─────────────────────────────────────────────────────────────── + + +def _from_document(path: Path, original_name: str) -> dict[str, Any]: + extension = extension_of(original_name) or extension_of(path.name) + + if extension == ".pdf": + return _from_pdf(path) + if extension == ".docx": + return _from_docx(path) + if extension == ".pptx": + return _from_pptx(path) + if extension == ".xlsx": + return _from_xlsx(path) + # .txt/.md/.csv carry no metadata, and the legacy formats extract_text already + # refuses by name are not readable here either. + return {} + + +def _from_pdf(path: Path) -> dict[str, Any]: + import pypdfium2 + + document = pypdfium2.PdfDocument(path) + try: + return _from_core_properties( + author=document.get_metadata_value("Author"), + title=document.get_metadata_value("Title"), + created=document.get_metadata_value("CreationDate"), + ) + finally: + document.close() + + +def _from_docx(path: Path) -> dict[str, Any]: + import docx + + properties = docx.Document(str(path)).core_properties + return _from_core_properties( + author=properties.author, + title=properties.title, + created=properties.created, + publisher=properties.company if hasattr(properties, "company") else None, + ) + + +def _from_pptx(path: Path) -> dict[str, Any]: + from pptx import Presentation + + properties = Presentation(str(path)).core_properties + return _from_core_properties( + author=properties.author, + title=properties.title, + created=properties.created, + ) + + +def _from_xlsx(path: Path) -> dict[str, Any]: + import openpyxl + + workbook = openpyxl.load_workbook(path, read_only=True, data_only=True) + try: + properties = workbook.properties + return _from_core_properties( + author=properties.creator, + title=properties.title, + created=properties.created, + ) + finally: + workbook.close() + + +def _from_core_properties( + *, + author: Any = None, + title: Any = None, + created: Any = None, + publisher: Any = None, +) -> dict[str, Any]: + """The shape every document format reduces to. + + Unlike a media container, a document's `title` *is* the work's own title, so it maps + to `source_title`. See the note on `_TAG_SOURCES` for why media goes the other way. + """ + found: dict[str, Any] = {} + + cleaned_author = _clean(author) + # Office writes "python-docx"-style generator names into `author` when nobody set + # one, and Word defaults it to the machine's registered owner. Neither is worth + # rejecting heuristically — the user can clear it, and a blank field teaches them + # nothing — but an empty-ish one is not worth writing either. + if cleaned_author: + found["creator"] = cleaned_author + + cleaned_title = _clean(title) + if cleaned_title: + found["source_title"] = cleaned_title + + cleaned_publisher = _clean(publisher) + if cleaned_publisher: + found["publisher"] = cleaned_publisher + + when = normalise_partial_date(created) + if when: + found["published_date"] = when + + return found + + +# ─── shared helpers ────────────────────────────────────────────────────────── + +# Leading year, then optionally month and day, separated by - or : (EXIF uses colons). +_DATE_HEAD = re.compile(r"^\s*(\d{4})(?:[-:/](\d{1,2}))?(?:[-:/](\d{1,2}))?") + +# PDF writes a separator-less date with a marker prefix: "D:20190315101112Z". Matched +# separately because the run-together digits are ambiguous under the pattern above — +# it would read the year and then stop, losing the month and day. +_DATE_COMPACT = re.compile(r"^(?:D:)?(\d{4})(\d{2})(\d{2})(?:\d{2})*[Zz+\-]?") + + +def normalise_partial_date(value: Any) -> Optional[str]: + """Reduce whatever a file claims as a date to `YYYY`, `YYYY-MM` or `YYYY-MM-DD`. + + Containers are wildly inconsistent here: MP4 writes an ISO instant + ("2019-03-15T10:00:00.000000Z"), ID3 often writes a bare year, EXIF uses colons + ("2019:03:15 10:00:00"), and Office hands back a real datetime object. All of them + reduce to the partial-date string `Asset.published_date` stores. + + Precision is never invented: a file that says only "2019" yields "2019", not + "2019-01-01". That is the entire reason the column is a string — see the comment on + the model field. + """ + if value is None: + return None + + if isinstance(value, datetime): + return value.strftime("%Y-%m-%d") + + raw = str(value).strip() + + compact = _DATE_COMPACT.match(raw) + if compact: + year, month, day = compact.groups() + return _assemble_date(year, month, day) + + match = _DATE_HEAD.match(raw.removeprefix("D:")) + if not match: + return None + + year, month, day = match.groups() + return _assemble_date(year, month, day) + + +def _assemble_date( + year: str, month: Optional[str], day: Optional[str] +) -> Optional[str]: + """Build the widest valid partial date these parts support. + + Degrades rather than rejects: a file claiming month 19 still knows its year, and + keeping the year is better than discarding the whole field over one bad component. + """ + if not 1000 <= int(year) <= 9999: + return None + if month is None: + return year + if not 1 <= int(month) <= 12: + return year + if day is None: + return f"{year}-{int(month):02d}" + if not 1 <= int(day) <= 31: + return f"{year}-{int(month):02d}" + return f"{year}-{int(month):02d}-{int(day):02d}" + + +def _looks_like_url(value: str) -> bool: + return value.lower().startswith(("http://", "https://")) + + +def _clean(value: Any) -> Optional[str]: + """Trim, and treat whitespace-only and Pillow's trailing NULs as absent.""" + if value is None: + return None + if isinstance(value, bytes): + value = value.decode("utf-8", errors="replace") + if not isinstance(value, str): + return None + cleaned = value.replace("\x00", "").strip() + return cleaned or None diff --git a/backend/app/ingest/probe.py b/backend/app/ingest/probe.py index cfc1e70..20230c6 100644 --- a/backend/app/ingest/probe.py +++ b/backend/app/ingest/probe.py @@ -11,9 +11,9 @@ import json import logging import subprocess -from dataclasses import dataclass +from dataclasses import dataclass, field from pathlib import Path -from typing import Any, Optional +from typing import Any, Mapping, Optional from app.media_tools import ffmpeg_available @@ -32,6 +32,14 @@ class ProbeResult: codec: Optional[str] = None has_audio: bool = False has_video: bool = False + # The container's own tag block — artist, copyright, date and friends. ffprobe has + # been returning these all along (`-show_format` includes them) and this class threw + # them away; M10's attribution harvester reads them. Keys are lowercased here + # because containers disagree about case and callers should not have to. + # + # `frozen=True` is for immutability, not hashability — a mutable default would be + # shared across instances, and nothing hashes a ProbeResult. + tags: Mapping[str, str] = field(default_factory=dict) def probe(path: Path) -> ProbeResult: @@ -97,9 +105,30 @@ def _interpret(payload: dict[str, Any]) -> ProbeResult: codec=codec, has_audio=audio is not None, has_video=video is not None, + tags=_container_tags(payload, video, audio), ) +def _container_tags( + payload: dict[str, Any], + video: Optional[dict[str, Any]], + audio: Optional[dict[str, Any]], +) -> Mapping[str, str]: + """The container's tag block, lowercased, format first and streams as a fallback. + + Where a tag lives depends on the container: MP4 and MP3 put artist/copyright/date on + the format, while some MKV and transport-stream files carry them only on a stream. + Reading both means the caller does not have to know which it was handed. Format wins, + because it describes the file rather than one track of it. + """ + merged: dict[str, str] = {} + for source in (audio, video, payload.get("format")): + for key, value in ((source or {}).get("tags") or {}).items(): + if isinstance(value, str) and value.strip(): + merged[str(key).strip().lower()] = value.strip() + return merged + + def _display_dimensions(stream: dict[str, Any]) -> tuple[Optional[int], Optional[int]]: """Width and height as the video will actually be shown. diff --git a/backend/app/jobs/enrichment.py b/backend/app/jobs/enrichment.py index ac83cb2..9a18ae9 100644 --- a/backend/app/jobs/enrichment.py +++ b/backend/app/jobs/enrichment.py @@ -18,7 +18,9 @@ from app.database import engine from app.embeddings import EmbeddingError, build_embedder from app.enrichment.embed import EmbeddingUnavailable +from app.enrichment.attribute import run as run_attribute from app.enrichment.backfill import run as run_backfill +from app.enrichment.harvest_attribution import run as run_harvest_attribution from app.enrichment.embed import run as run_embed from app.enrichment.autotag import run as run_autotag from app.enrichment.bulk import run as run_bulk @@ -44,7 +46,9 @@ from app.models.job import ( EnrichmentJob, KIND_AUTOTAG, + KIND_ATTRIBUTE, KIND_BACKFILL_EMBEDDINGS, + KIND_HARVEST_ATTRIBUTION, KIND_BULK_ENRICH, KIND_DESCRIBE, KIND_EMBED, @@ -252,6 +256,8 @@ def _run_job(job_id: str) -> None: detail = run_summarize(session, asset, progress) elif job.kind == KIND_AUTOTAG: detail = run_autotag(session, asset, progress) + elif job.kind == KIND_ATTRIBUTE: + detail = run_attribute(session, asset, progress) elif job.kind == KIND_DESCRIBE: detail = run_describe(session, asset, progress) elif job.kind == KIND_GENERATE_ALL: @@ -272,6 +278,14 @@ def _run_job(job_id: str) -> None: # Surfaced rather than swallowed: a run that quietly skipped three # assets looks identical to one that embedded everything. detail += f", {result.failed} failed" + elif job.kind == KIND_HARVEST_ATTRIBUTION: + harvested = run_harvest_attribution(session, job.user_id, progress) + detail = ( + f"{harvested.attributed} attributed of {harvested.scanned} scanned" + ) + if harvested.failed: + # Surfaced rather than swallowed, same as the two runs above. + detail += f", {harvested.failed} unreadable" else: raise TranscriptionError(f"Unknown enrichment kind: {job.kind}") diff --git a/backend/app/models/asset.py b/backend/app/models/asset.py index 751c06b..68d5bd6 100644 --- a/backend/app/models/asset.py +++ b/backend/app/models/asset.py @@ -8,6 +8,20 @@ from app.clock import utcnow +# Who last wrote a field, as recorded in `Asset.field_provenance`. +# +# "embedded" is a third value beside the original two, and the distinction it draws is +# load-bearing: a tag read out of a file (EXIF Artist, an ID3 frame, a PDF Author) is a +# fact about the file but not a claim anybody checked — it is frequently the camera +# owner, a studio default, or boilerplate. Keeping it separate from "human" is what lets +# a later pass propose over a camera-supplied name while never proposing over something +# the user typed. Collapse the two and that distinction is gone for good, because +# nothing else records it. +PROVENANCE_HUMAN = "human" +PROVENANCE_AI = "ai" +PROVENANCE_EMBEDDED = "embedded" + + def new_asset_id() -> str: return str(uuid.uuid4()) @@ -86,8 +100,36 @@ class Asset(SQLModel, table=True): transcript_model: Optional[str] = None transcript_language: Optional[str] = None + # ─── attribution (M10) ─────────────────────────────────────────────────── + # Whose work this is, as opposed to `source` above, which is how the file got here. + # The two get conflated constantly; they answer different questions and neither + # substitutes for the other. Specified in docs/m10-attribution.md. + source_url: Optional[str] = None + creator: Optional[str] = None # author, photographer, speaker, director + publisher: Optional[str] = None # outlet, channel, studio, imprint + source_title: Optional[str] = None # the programme, film, article or book + # A string, not a datetime, against this file's own convention two blocks down — + # and deliberately. Publication dates are routinely partial: a book is from 1994, a + # magazine piece from March 2019. A datetime cannot hold either without inventing a + # January 1st that then reads as a real one, which in a citation record is exactly + # the quiet falsehood this milestone exists to prevent. Stores ISO 8601 `YYYY`, + # `YYYY-MM` or `YYYY-MM-DD`, validated on write in schemas_assets.AssetUpdate. + # ISO partial dates compare correctly as plain strings ("2018-12-31" < "2019" < + # "2019-03-01"), so the date filters need no parsing and no special cases. + published_date: Optional[str] = Field(default=None, index=True) + # A real datetime, because a download happened at an instant — there is no + # partial-precision case here to serve. + retrieved_at: Optional[datetime] = None + license: Optional[str] = None + # The displayed citation, and an *override* only — null until somebody types one. + # The value shown is composed from the fields above on read (app/attribution.py). + # Storing the composition instead would leave it stale the moment `publisher` is + # corrected, which is the same rot that made copy-on-create the wrong answer for a + # clip's inherited attribution one level down. + credit_line: Optional[str] = None + # ─── provenance of the metadata, not the file ─────────────────────────── - # JSON, {"description": "ai"|"human", ...}. FR 8.1.3 requires that a later AI run + # JSON, {"description": "ai"|"human"|"embedded", ...}. FR 8.1.3 requires that a later AI run # never silently overwrites something a person wrote; without recording who last # wrote each field, that rule has nothing to check against. Written from M6, read # never before — but the column exists now so the first enrichment run has diff --git a/backend/app/models/job.py b/backend/app/models/job.py index 7baace3..1a8d3af 100644 --- a/backend/app/models/job.py +++ b/backend/app/models/job.py @@ -45,6 +45,15 @@ # costs nothing, the same reason `KIND_EXTRACT_TEXT` sits apart from the LLM jobs # despite also being per-asset. KIND_EXTRACT_SUBVIDEO = "extract_subvideo" +# M10. Proposing where an asset came from. Inside `ENRICHMENT_KINDS` unlike the two +# above: it calls a provider and costs money, so it belongs in the bulk-enrichment +# selection UI and its cost estimate. +KIND_ATTRIBUTE = "attribute" +# M10. Re-reading embedded attribution (EXIF, ID3, PDF Author) across a whole library, +# for the files that were already there when M10 landed. Library-wide, and outside +# `ENRICHMENT_KINDS` for the same reason as the two above: it calls no provider and +# costs nothing. +KIND_HARVEST_ATTRIBUTION = "harvest_attribution" # Per-asset actions. Everything in here requires an `asset_id`. ENRICHMENT_KINDS = frozenset( @@ -56,6 +65,7 @@ KIND_EMBED, KIND_EXTRACT_TEXT, KIND_GENERATE_ALL, + KIND_ATTRIBUTE, } ) @@ -63,7 +73,9 @@ # branches on this set rather than on a hardcoded kind, so adding another one needs no # change to the dispatch. A bulk run over a selection is here too: it has many assets, # which for the purposes of `asset_id` is the same as having none. -LIBRARY_KINDS = frozenset({KIND_BACKFILL_EMBEDDINGS, KIND_BULK_ENRICH}) +LIBRARY_KINDS = frozenset( + {KIND_BACKFILL_EMBEDDINGS, KIND_BULK_ENRICH, KIND_HARVEST_ATTRIBUTION} +) class EnrichmentJob(SQLModel, table=True): diff --git a/backend/app/models/suggestion.py b/backend/app/models/suggestion.py index da374b0..906b535 100644 --- a/backend/app/models/suggestion.py +++ b/backend/app/models/suggestion.py @@ -29,7 +29,19 @@ # writing it replaces something rather than filling a blank. KIND_TAG = "tag" KIND_TITLE = "title" -KINDS = (KIND_TAG, KIND_TITLE) +# M10. One row per proposed attribution field, with `value` holding JSON-as-TEXT: +# {"field": "publisher", "value": "BBC Two", "evidence": "the chyron reads BBC TWO"}. +# +# One kind rather than eight, because `accept` dispatches on `kind` and eight near +# identical branches would be eight places to forget one. Still one row per field, so a +# user can take the publisher and decline the date — which is the common case, since a +# model reads a channel logo far more reliably than a broadcast date. +# +# `evidence` is not decoration: attribution is the one enrichment that may not write +# directly, because a fabricated citation is worse than a blank one, and a reviewer who +# cannot see what the model read is not really reviewing anything. +KIND_ATTRIBUTION = "attribution" +KINDS = (KIND_TAG, KIND_TITLE, KIND_ATTRIBUTION) STATUS_PENDING = "pending" STATUS_ACCEPTED = "accepted" diff --git a/backend/app/routers/assets.py b/backend/app/routers/assets.py index c9864c7..06e6e20 100644 --- a/backend/app/routers/assets.py +++ b/backend/app/routers/assets.py @@ -12,10 +12,14 @@ from app.auth import CurrentUser from app.database import get_session from app.ingest.filetypes import ASSET_TYPES +from app.jobs import enrichment as enrichment_jobs +from app.jobs.registry import KINDS from app.models.asset import Asset +from app.models.job import KIND_HARVEST_ATTRIBUTION from app.models.tag import Tag from app.schemas import DataResponse, ListResponse from app.schemas_assets import AssetRead, AssetUpdate, UploadRejection, UploadResult +from app.schemas_jobs import ActivityJobRead from app.schemas_tags import AssetTagsWrite, BulkTagsResult, BulkTagsWrite, TagRead from app.services import assets as service from app.services import tags as tag_service @@ -136,6 +140,12 @@ def list_assets( max_duration: Optional[float] = Query(default=None, ge=0), uploaded_after: Optional[datetime] = Query(default=None), uploaded_before: Optional[datetime] = Query(default=None), + creator: Optional[str] = Query(default=None, max_length=200), + publisher: Optional[str] = Query(default=None, max_length=200), + source_title: Optional[str] = Query(default=None, max_length=200), + published_after: Optional[str] = Query(default=None, max_length=10), + published_before: Optional[str] = Query(default=None, max_length=10), + unattributed: Optional[bool] = Query(default=None), limit: int = Query(default=50, ge=1, le=MAX_PAGE_SIZE), offset: int = Query(default=0, ge=0), session: Session = Depends(get_session), @@ -186,6 +196,41 @@ def list_assets( | col(Asset.original_name).ilike(term) ) + # Attribution (M10). Each resolves through the parent the same way the read model + # does, so a clip is returned by the publisher it inherited — see + # `services/assets.own_or_inherited` for why a row-only filter would be wrong. + for field_name, value in ( + ("creator", creator), + ("publisher", publisher), + ("source_title", source_title), + ): + if value and value.strip(): + needle = f"%{value.strip()}%" + filters.append( + service.own_or_inherited( + field_name, lambda column, n=needle: column.ilike(n) + ) + ) + + # Lexicographic, which is chronological here because AssetUpdate guarantees the + # format. "2019" as a lower bound therefore includes all of 2019, and as an upper + # bound excludes it — the same half-open behaviour a date picker implies. + if published_after: + filters.append( + service.own_or_inherited( + "published_date", lambda column, v=published_after: column >= v + ) + ) + if published_before: + filters.append( + service.own_or_inherited( + "published_date", lambda column, v=published_before: column <= v + ) + ) + + if unattributed is not None: + filters.append(service.unattributed_clause(unattributed)) + # Tag and category filters resolve to a set of ids first. Both are questions about # the join table rather than about the asset row, and an id set keeps them from # turning the main query into a pile of correlated subqueries — one per tag, in the @@ -425,3 +470,35 @@ def bulk_tag_assets( return DataResponse[BulkTagsResult]( data=BulkTagsResult(updated=len(owned), tags_added=[_tag_read(t) for t in added]) ) + + +@router.post( + "/harvest-attribution", + response_model=DataResponse[ActivityJobRead], + status_code=status.HTTP_202_ACCEPTED, +) +def start_attribution_harvest( + user: CurrentUser, session: Session = Depends(get_session) +) -> DataResponse[ActivityJobRead]: + """Re-read embedded attribution for every file already in this library. + + Ingest harvests each new upload, so this exists for everything uploaded before M10 — + on an instance that has been running a while, the whole library. Those files have + been carrying their EXIF and ID3 and PDF Author on disk the entire time; nothing has + ever looked. + + Safe to run repeatedly, because the harvest only ever fills blanks: a second run is a + no-op over what the first one filled and over anything since corrected by hand. That + is also why there is no "already harvested" flag to keep. + """ + if enrichment_jobs.active_library_job(session, user.id, KIND_HARVEST_ATTRIBUTION): + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail={ + "code": "already_running", + "message": "Your library is already being scanned for embedded metadata", + }, + ) + + job = enrichment_jobs.submit_library(session, user.id, KIND_HARVEST_ATTRIBUTION) + return DataResponse(data=KINDS["enrichment"].to_activity(job)) diff --git a/backend/app/routers/enrichment.py b/backend/app/routers/enrichment.py index da4ffbc..063dcc5 100644 --- a/backend/app/routers/enrichment.py +++ b/backend/app/routers/enrichment.py @@ -25,6 +25,7 @@ from app.jobs.registry import KINDS from app.models.asset import Asset from app.models.job import ( + KIND_ATTRIBUTE, KIND_AUTOTAG, KIND_BULK_ENRICH, KIND_DESCRIBE, @@ -34,7 +35,7 @@ KIND_SUMMARIZE, ) from app.models.document import DocumentPage -from app.models.suggestion import Suggestion +from app.models.suggestion import KIND_ATTRIBUTION, Suggestion from app.providers import build_provider from app.schemas import DataResponse, ListResponse from app.schemas_jobs import ActivityJobRead @@ -52,6 +53,25 @@ class SuggestionRead(BaseModel): value: str status: str + # M10. Decoded here rather than in the browser: a `kind == "attribution"` row carries + # JSON in `value`, and asking the client to parse a server-side encoding would put + # the same shape in two places to keep in step. Null on every other kind. + field: Optional[str] = None + proposed_value: Optional[str] = None + # What the model quoted as its reason. The grounding rule is the reason attribution + # is suggestion-only, and a reviewer who cannot see what it read is not reviewing. + evidence: Optional[str] = None + + +def _to_suggestion_read(row: Suggestion) -> SuggestionRead: + read = SuggestionRead.model_validate(row) + decoded = suggestion_service.decode_attribution(row) if row.kind == KIND_ATTRIBUTION else {} + if decoded: + read.field = decoded["field"] + read.proposed_value = decoded["value"] + read.evidence = decoded["evidence"] + return read + def _require_provider(session: Session, user_id: str) -> None: """Refuse before queueing work that is certain to fail. @@ -119,6 +139,46 @@ def start_summarize( return DataResponse(data=KINDS["enrichment"].to_activity(job)) +@router.post( + "/{asset_id}/attribute", response_model=DataResponse[ActivityJobRead], status_code=202 +) +def start_attribute( + asset_id: str, + user: CurrentUser, + session: Session = Depends(get_session), +) -> DataResponse[ActivityJobRead]: + """Queue a pass that proposes where this came from. It writes nothing. + + The one enrichment that may never write directly: a description the model gets wrong + is a poorer search result, while a citation it gets wrong credits somebody else's + work to the wrong outlet, in a field that then looks finished. Every proposal arrives + as a `Suggestion` carrying the evidence it was read from, for a person to accept. + """ + 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 read", + }, + ) + + if enrichment_jobs.active_job(session, asset.id, KIND_ATTRIBUTE) is not None: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail={ + "code": "already_running", + "message": "This asset is already being attributed", + }, + ) + + job = enrichment_jobs.submit(session, asset, KIND_ATTRIBUTE) + return DataResponse(data=KINDS["enrichment"].to_activity(job)) + + @router.post("/{asset_id}/autotag", response_model=DataResponse[ActivityJobRead], status_code=202) def start_autotag( asset_id: str, @@ -307,7 +367,7 @@ def list_suggestions( _owned_asset(asset_id, user.id, session) rows = suggestion_service.pending_for(session, asset_id) return ListResponse( - data=[SuggestionRead.model_validate(r) for r in rows], + data=[_to_suggestion_read(r) for r in rows], total=len(rows), limit=len(rows), offset=0, @@ -340,7 +400,7 @@ def accept_suggestion( asset = _owned_asset(asset_id, user.id, session) row = _owned_suggestion(asset_id, suggestion_id, user.id, session) return DataResponse( - data=SuggestionRead.model_validate(suggestion_service.accept(session, asset, row)) + data=_to_suggestion_read(suggestion_service.accept(session, asset, row)) ) @@ -357,7 +417,7 @@ def reject_suggestion( """Record a no. Kept, so a later run does not propose the same thing again.""" _owned_asset(asset_id, user.id, session) row = _owned_suggestion(asset_id, suggestion_id, user.id, session) - return DataResponse(data=SuggestionRead.model_validate(suggestion_service.reject(session, row))) + return DataResponse(data=_to_suggestion_read(suggestion_service.reject(session, row))) class BulkEnrichRequest(BaseModel): diff --git a/backend/app/schemas_assets.py b/backend/app/schemas_assets.py index 22abd36..999097c 100644 --- a/backend/app/schemas_assets.py +++ b/backend/app/schemas_assets.py @@ -2,11 +2,17 @@ from __future__ import annotations +import re from datetime import datetime from typing import List, Optional from pydantic import BaseModel, Field, field_validator +# A whole year, a year and month, or a full date. Months 01-12 and days 01-31 are +# enforced here so the string column cannot hold "2019-13" and sort between "2019-12" +# and "2020-01" — the range filters rely on lexicographic order being chronological. +_ISO_PARTIAL_DATE = re.compile(r"\d{4}(?:-(?:0[1-9]|1[0-2])(?:-(?:0[1-9]|[12]\d|3[01]))?)?") + class AssetRead(BaseModel): id: str @@ -42,6 +48,29 @@ class AssetRead(BaseModel): # vanished underneath the database shows as missing instead of as a broken image. missing: bool = False + # ─── attribution (M10) ─────────────────────────────────────────────────── + # These are the *resolved* values: a clip with nothing of its own carries what it + # inherited from its parent, so the panel shows a real credit rather than eight + # blanks beside a video that is plainly attributed. + source_url: Optional[str] = None + creator: Optional[str] = None + publisher: Optional[str] = None + source_title: Optional[str] = None + published_date: Optional[str] = None + retrieved_at: Optional[datetime] = None + license: Optional[str] = None + credit_line: Optional[str] = None + + # The line to display: `credit_line` when one was typed, otherwise composed from the + # fields above. Read-only and computed per response, for the same reason `file_url` + # is: a stored copy would be stale the moment a component field changed. + credit: str = "" + # Which of the fields above came from the parent rather than from this row. Sent so + # the UI can mark an inherited value as inherited instead of letting it look like + # something typed on the clip — which is what would make a user "correct" it here + # and quietly break the link to the source. + attribution_inherited: List[str] = Field(default_factory=list) + # Batch-loaded for a listing, never per row: sixty assets a page each asking for # their own tags is sixty queries that grow with the page. tags: List["AssetTagRead"] = Field(default_factory=list) @@ -74,6 +103,36 @@ class AssetUpdate(BaseModel): description: Optional[str] = Field(default=None, max_length=20_000) summary: Optional[str] = Field(default=None, max_length=20_000) + # Attribution (M10). All editable by hand — the harvester only ever fills blanks, + # and an AI may only propose, so this is the sole path that can correct a value. + source_url: Optional[str] = Field(default=None, max_length=2_000) + creator: Optional[str] = Field(default=None, max_length=500) + publisher: Optional[str] = Field(default=None, max_length=500) + source_title: Optional[str] = Field(default=None, max_length=500) + published_date: Optional[str] = Field(default=None, max_length=10) + retrieved_at: Optional[datetime] = None + license: Optional[str] = Field(default=None, max_length=500) + credit_line: Optional[str] = Field(default=None, max_length=2_000) + + @field_validator("published_date") + @classmethod + def _published_date_is_an_iso_partial(cls, value: Optional[str]) -> Optional[str]: + """`YYYY`, `YYYY-MM` or `YYYY-MM-DD`, and nothing else. + + The looseness of a string column is what lets a partial date exist at all; the + validator is what stops it becoming a free-text field where "summer 1994" and + "15/03/19" would sort meaninglessly and break the range filters, which compare + lexicographically precisely because the format is guaranteed here. + """ + if value is None: + return None + cleaned = value.strip() + if not cleaned: + return None + if not _ISO_PARTIAL_DATE.fullmatch(cleaned): + raise ValueError("published_date must be YYYY, YYYY-MM or YYYY-MM-DD") + return cleaned + @field_validator("name") @classmethod def _name_is_not_blank(cls, value: Optional[str]) -> Optional[str]: diff --git a/backend/app/search/fts.py b/backend/app/search/fts.py index d6f8713..54640d3 100644 --- a/backend/app/search/fts.py +++ b/backend/app/search/fts.py @@ -46,6 +46,11 @@ # UNINDEXED columns ride along so a hit resolves to an asset and a timestamp without a # second query, while staying out of the tokeniser — an asset id must not match a text # search. +# +# `attribution_text` is appended rather than slotted in beside `name`, and that position +# is load-bearing: `search_assets` passes a *column index* to snippet() (2, for `name`), +# so inserting a column anywhere before it would silently start excerpting the wrong +# field. Appending keeps every existing index valid. ASSET_FTS_DDL = """ CREATE VIRTUAL TABLE asset_fts USING fts5( asset_id UNINDEXED, @@ -54,6 +59,7 @@ description, summary, tags_text, + attribution_text, tokenize='porter unicode61' ) """ @@ -69,7 +75,15 @@ ) """ -ASSET_FTS_COLUMNS = ("asset_id", "user_id", "name", "description", "summary", "tags_text") +ASSET_FTS_COLUMNS = ( + "asset_id", + "user_id", + "name", + "description", + "summary", + "tags_text", + "attribution_text", +) SEGMENT_FTS_COLUMNS = ("segment_id", "asset_id", "user_id", "body", "start_time") # The virtual tables, plus the shadow tables SQLite creates behind each one @@ -119,13 +133,22 @@ class FtsHit: # ─── writing ───────────────────────────────────────────────────────────────── -def index_asset(session: Session, asset: Asset, tags_text: str = "") -> None: - """Re-index one asset's own metadata. Safe to call repeatedly.""" +def index_asset( + session: Session, asset: Asset, tags_text: str = "", attribution_text: str = "" +) -> None: + """Re-index one asset's own metadata. Safe to call repeatedly. + + `attribution_text` is passed in rather than composed here for the same reason + `tags_text` is: this module knows about the index, not about what the application + considers worth indexing. `services/assets._reindex` builds both. + """ remove_asset(session, asset.id) session.execute( text( - f"INSERT INTO {ASSET_FTS} (asset_id, user_id, name, description, summary, tags_text)" - " VALUES (:asset_id, :user_id, :name, :description, :summary, :tags_text)" + f"INSERT INTO {ASSET_FTS}" + " (asset_id, user_id, name, description, summary, tags_text, attribution_text)" + " VALUES" + " (:asset_id, :user_id, :name, :description, :summary, :tags_text, :attribution_text)" ), { "asset_id": asset.id, @@ -134,6 +157,7 @@ def index_asset(session: Session, asset: Asset, tags_text: str = "") -> None: "description": asset.description or "", "summary": asset.summary or "", "tags_text": tags_text, + "attribution_text": attribution_text, }, ) session.commit() diff --git a/backend/app/services/assets.py b/backend/app/services/assets.py index 2d3791c..ffebaa8 100644 --- a/backend/app/services/assets.py +++ b/backend/app/services/assets.py @@ -10,14 +10,17 @@ import json import logging import mimetypes -from typing import AsyncIterator, Iterable, Optional +from typing import Any, AsyncIterator, Iterable, Optional +from sqlalchemy import and_, func, not_, or_ +from sqlalchemy.orm import aliased from sqlmodel import Session, col, delete, select, update +from app import attribution from app.auth import sign_media_key from app.clock import utcnow from app.config import settings -from app.ingest import thumbnails +from app.ingest import embedded_metadata, thumbnails from app.ingest.filetypes import ( SOURCE_CLIP, SOURCE_SUBVIDEO, @@ -31,7 +34,12 @@ sanitize_original_name, ) from app.ingest.probe import ProbeResult, probe -from app.models.asset import Asset +from app.models.asset import ( + PROVENANCE_AI, + PROVENANCE_EMBEDDED, + PROVENANCE_HUMAN, + Asset, +) from app.models.suggestion import Suggestion from app.models.document import DocumentPage from app.models.transcript import TranscriptSegment @@ -106,6 +114,7 @@ def _describe(session: Session, storage: LocalStorage, asset: Asset) -> None: if not asset.storage_key: return + embedded: dict = {} try: with storage.materialise(asset.storage_key) as path: result = probe(path) @@ -129,6 +138,15 @@ def _describe(session: Session, storage: LocalStorage, asset: Asset) -> None: asset.original_name or "", duration_seconds=result.duration_seconds, ) + + # Read inside the `with`, applied after: the file is only on disk for the + # duration of this block, but writing needs a committed row. + embedded = embedded_metadata.harvest( + path, + asset.asset_type, + asset.original_name or "", + probe_tags=result.tags, + ) except (StorageError, OSError) as exc: logger.warning("Could not describe asset %s: %s", asset.id, exc) return @@ -144,6 +162,37 @@ def _describe(session: Session, storage: LocalStorage, asset: Asset) -> None: session.commit() session.refresh(asset) _reindex(session, asset) + apply_embedded_attribution(session, asset, embedded) + + +def apply_embedded_attribution(session: Session, asset: Asset, embedded: dict) -> None: + """Write harvested attribution into the fields that are still empty. + + Filtering to empty fields is what makes this safe to run without asking, and what + makes the library-wide re-harvest safe to run twice: an EXIF `Artist` is a fact about + the file, but it is frequently the camera's registered owner rather than the + photographer, so it gets to fill a blank and never to overwrite an answer. The + `PROVENANCE_EMBEDDED` stamp is what lets a later pass tell the two apart. + """ + if not embedded: + return + + fillable = { + name: value + for name, value in embedded.items() + if name in attribution.ATTRIBUTION_FIELDS and _is_unset(getattr(asset, name, None)) + } + if not fillable: + return + + try: + apply_embedded_metadata(session, asset, fillable) + except Exception: # noqa: BLE001 - an upload that succeeded must stay succeeded + logger.warning("Could not store embedded attribution for %s", asset.id, exc_info=True) + + +def _is_unset(value) -> bool: + return value is None or (isinstance(value, str) and not value.strip()) def _chain_transcription(session: Session, asset: Asset) -> None: @@ -201,7 +250,20 @@ def _reindex(session: Session, asset: Asset) -> None: whether the write succeeded. """ try: - fts.index_asset(session, asset, tags_text=tags.tags_text_for(session, asset.id)) + # Attribution is resolved through the parent before indexing, so a clip is + # findable by the publisher it inherited and "everything from the BBC" is not + # quietly missing every clip. The parent lookup only happens for rows that have + # one, and `_reindex_children_of` below keeps those entries current when the + # parent's own attribution later changes. + parent = ( + session.get(Asset, asset.parent_asset_id) if asset.parent_asset_id else None + ) + fts.index_asset( + session, + asset, + tags_text=tags.tags_text_for(session, asset.id), + attribution_text=attribution.index_text(attribution.resolve(asset, parent)), + ) except Exception: # noqa: BLE001 - see above logger.warning("Could not index asset %s for search", asset.id, exc_info=True) @@ -298,25 +360,7 @@ def apply_metadata(session: Session, asset: Asset, changes: dict) -> Asset: `summary` the stamp is now informational only — `apply_ai_metadata` no longer reads it before overwriting either field. """ - if not changes: - return asset - - try: - provenance = json.loads(asset.field_provenance or "{}") - except ValueError: - provenance = {} - - for field, value in changes.items(): - setattr(asset, field, value) - provenance[field] = "human" - - asset.field_provenance = json.dumps(provenance, sort_keys=True) - asset.metadata_modified_date = utcnow() - - session.add(asset) - session.commit() - session.refresh(asset) - _reindex(session, asset) + _apply(session, asset, changes, PROVENANCE_HUMAN) return asset @@ -331,17 +375,45 @@ def apply_ai_metadata(session: Session, asset: Asset, changes: dict) -> list[str Returns the fields written (always every key in `changes`, once any are given). """ + _apply(session, asset, changes, PROVENANCE_AI) + return list(changes.keys()) + + +def apply_embedded_metadata(session: Session, asset: Asset, changes: dict) -> list[str]: + """Write fields read out of the file's own container metadata. + + The third writer, and the reason `_apply` takes the stamp as an argument rather than + hard-coding one. An EXIF `Artist` or an ID3 frame is a fact about the file, so it is + written without asking — but it is not something a person verified, and + `PROVENANCE_EMBEDDED` is what preserves that difference for whatever reads it later. + + Callers are responsible for having filtered to fields that are actually empty: this + overwrites like the other two, because a writer that silently skipped would hide the + same class of bug `apply_ai_metadata`'s docstring describes. + """ + _apply(session, asset, changes, PROVENANCE_EMBEDDED) + return list(changes.keys()) + + +def _apply(session: Session, asset: Asset, changes: dict, provenance_value: str) -> None: + """The shared body of the three writers above: set the fields, stamp who set them. + + A malformed `field_provenance` is rebuilt rather than raising. It is a record *about* + the metadata, and losing that record must never cost the write it describes. + """ if not changes: - return [] + return try: provenance = json.loads(asset.field_provenance or "{}") except ValueError: provenance = {} + if not isinstance(provenance, dict): + provenance = {} for field_name, value in changes.items(): setattr(asset, field_name, value) - provenance[field_name] = "ai" + provenance[field_name] = provenance_value asset.field_provenance = json.dumps(provenance, sort_keys=True) asset.metadata_modified_date = utcnow() @@ -350,7 +422,31 @@ def apply_ai_metadata(session: Session, asset: Asset, changes: dict) -> list[str session.commit() session.refresh(asset) _reindex(session, asset) - return list(changes.keys()) + + # A clip's index entry carries the attribution it inherited, so correcting this + # asset's publisher leaves every clip of it indexed under the old one until they are + # rebuilt. Only on an attribution change, and only for rows that actually have + # children — the common edit (a description, a summary) touches neither. + if any(name in changes for name in attribution.ATTRIBUTION_FIELDS): + _reindex_children_of(session, asset) + + +def _reindex_children_of(session: Session, parent: Asset) -> None: + """Rebuild the index entries of everything derived from this asset. + + Inheritance is resolved at read time everywhere *except* the keyword index, which by + its nature stores a snapshot. This is the one place that snapshot has to be caught + up, and it is why `resolve`'s one-level rule matters: there is no grandchild to + recurse into. + """ + try: + children = list_children(session, parent.id, parent.user_id) + except Exception: # noqa: BLE001 - a stale child index must not fail the parent's write + logger.warning("Could not list children of %s to re-index", parent.id, exc_info=True) + return + + for child in children: + _reindex(session, child) # ─── clips and sub-videos (M7) ──────────────────────────────────────────────── @@ -537,6 +633,64 @@ def parents_for_many(session: Session, assets: Iterable[Asset]) -> dict[str, Ass return {row.id: row for row in rows} +def _blank(column): + """SQL for "nothing recorded here" — NULL, or an empty string. + + `retrieved_at` is a DateTime, where `!= ''` is not a meaningful comparison, so the + empty-string half is only applied to the text columns. + """ + if column.key == "retrieved_at": + return column.is_(None) + return or_(column.is_(None), func.trim(column) == "") + + +def _records_attribution(model) -> Any: + """SQL for "this row has at least one attribution field filled in".""" + return or_(*[not_(_blank(getattr(model, name))) for name in attribution.ATTRIBUTION_FIELDS]) + + +def own_or_inherited(field_name: str, build) -> Any: + """Lift a condition on an asset's own column to "or the parent it inherits from". + + Inheritance is resolved on read everywhere else, so a filter that only looked at the + row would answer "everything from the BBC" with the documentary and none of the + clips cut from it — which reads as a bug and is the sort of quiet incompleteness a + user has no way to notice. + + `build` turns a column into a condition. It is applied to the asset's own column and + to the parent's; the parent only counts where the asset's own value is blank, which + is exactly the coalesce `attribution.resolve` performs on read. + """ + parent = aliased(Asset) + own_column = getattr(Asset, field_name) + return or_( + build(own_column), + and_( + _blank(own_column), + select(1) + .where(parent.id == Asset.parent_asset_id, build(getattr(parent, field_name))) + .exists(), + ), + ) + + +def unattributed_clause(unattributed: bool) -> Any: + """Rows with no attribution at all, their parent's included. + + This is how a backlog gets worked through — "what still has no source" — so it has to + agree with what the panel shows. A clip that displays its parent's credit is not + missing one. + """ + parent = aliased(Asset) + inherits_attribution = ( + select(1) + .where(parent.id == Asset.parent_asset_id, _records_attribution(parent)) + .exists() + ) + anything = or_(_records_attribution(Asset), inherits_attribution) + return not_(anything) if unattributed else anything + + def to_read_model( asset: Asset, storage: LocalStorage, @@ -564,6 +718,8 @@ def to_read_model( if playable_key: missing = not storage.stat(playable_key).exists + resolved_attribution = attribution.resolve(asset, parent) + return AssetRead( id=asset.id, name=asset.name, @@ -585,6 +741,20 @@ def to_read_model( file_url=file_url, thumb_url=thumb_url, missing=missing, + # Resolved, not raw: a clip shows what it inherited, and `attribution_inherited` + # tells the UI which of those to mark as coming from the parent. `parent` is the + # same batch-loaded row the file_url resolution above already uses, so this adds + # no query to a listing. + source_url=resolved_attribution.source_url, + creator=resolved_attribution.creator, + publisher=resolved_attribution.publisher, + source_title=resolved_attribution.source_title, + published_date=resolved_attribution.published_date, + retrieved_at=resolved_attribution.retrieved_at, + license=resolved_attribution.license, + credit_line=resolved_attribution.credit_line, + credit=resolved_attribution.credit, + attribution_inherited=list(resolved_attribution.inherited), tags=[ AssetTagRead(id=t.id, name=t.name, category_id=t.category_id) for t in (asset_tags or []) diff --git a/backend/app/services/suggestions.py b/backend/app/services/suggestions.py index 19807b5..8580e25 100644 --- a/backend/app/services/suggestions.py +++ b/backend/app/services/suggestions.py @@ -8,14 +8,17 @@ from __future__ import annotations +import json import logging from typing import Iterable, Optional from sqlmodel import Session, col, delete, select +from app import attribution from app.clock import utcnow from app.models.asset import Asset from app.models.suggestion import ( + KIND_ATTRIBUTION, KIND_TAG, KIND_TITLE, STATUS_ACCEPTED, @@ -75,10 +78,14 @@ def propose( Only *pending* rows are cleared. Accepted and rejected ones are decisions the user made and this has no business discarding them. """ + # Scoped to the kinds this run produces. Clearing every pending row would throw away + # the attribution suggestions a separate job proposed, which the user has not seen + # yet and this run knows nothing about. session.exec( delete(Suggestion).where( col(Suggestion.asset_id) == asset.id, col(Suggestion.status) == STATUS_PENDING, + col(Suggestion.kind).in_([KIND_TAG, KIND_TITLE]), ) ) @@ -148,6 +155,15 @@ def accept(session: Session, asset: Asset, suggestion: Suggestion) -> Suggestion asset_service.reindex_ids(session, [asset.id]) elif suggestion.kind == KIND_TITLE: asset_service.apply_metadata(session, asset, {"name": suggestion.value}) + elif suggestion.kind == KIND_ATTRIBUTION: + decoded = decode_attribution(suggestion) + if decoded: + # Through `apply_metadata`, the *human* path, stamping provenance "human" — + # because it is: the user read the evidence and chose it. Same reasoning as + # the title above. + asset_service.apply_metadata( + session, asset, {decoded["field"]: decoded["value"]} + ) suggestion.status = STATUS_ACCEPTED suggestion.resolved_at = utcnow() @@ -168,3 +184,119 @@ def reject(session: Session, suggestion: Suggestion) -> Suggestion: session.commit() session.refresh(suggestion) return suggestion + + +# ─── attribution (M10) ─────────────────────────────────────────────────────── + + +def decode_attribution(suggestion: Suggestion) -> dict: + """The {field, value, evidence} a KIND_ATTRIBUTION row carries. + + Returns an empty dict rather than raising on anything malformed: a suggestion whose + payload cannot be read is one the UI should skip, not one that should break the + panel listing every other suggestion beside it. + """ + try: + payload = json.loads(suggestion.value or "{}") + except ValueError: + return {} + if not isinstance(payload, dict): + return {} + + field_name = payload.get("field") + value = payload.get("value") + if not isinstance(field_name, str) or field_name not in attribution.ATTRIBUTION_FIELDS: + return {} + if not isinstance(value, str) or not value.strip(): + return {} + + evidence = payload.get("evidence") + return { + "field": field_name, + "value": value.strip(), + "evidence": evidence.strip() if isinstance(evidence, str) else "", + } + + +def _declined_attribution(session: Session, asset_id: str) -> set[tuple[str, str]]: + """(field, value) pairs already refused, so a re-run does not ask twice.""" + rows = session.exec( + select(Suggestion).where( + Suggestion.asset_id == asset_id, + Suggestion.status == STATUS_REJECTED, + Suggestion.kind == KIND_ATTRIBUTION, + ) + ).all() + + declined = set() + for row in rows: + decoded = decode_attribution(row) + if decoded: + declined.add((decoded["field"], decoded["value"].lower())) + return declined + + +def propose_attribution( + session: Session, + asset: Asset, + *, + proposals: Iterable[dict], + model: str, +) -> int: + """Record proposed attribution, one row per field. + + Skips any field the asset already has a value for. A model reading a chyron is a + useful second opinion on a blank field and an unwelcome one on a field somebody + typed — and since accepting writes through the human path, letting it propose over + an answer would make "accept" a way to quietly overwrite one. + """ + session.exec( + delete(Suggestion).where( + col(Suggestion.asset_id) == asset.id, + col(Suggestion.status) == STATUS_PENDING, + col(Suggestion.kind) == KIND_ATTRIBUTION, + ) + ) + + declined = _declined_attribution(session, asset.id) + + created = 0 + seen: set[str] = set() + for proposal in proposals: + field_name = proposal.get("field") + value = (proposal.get("value") or "").strip() + evidence = (proposal.get("evidence") or "").strip() + + if field_name not in attribution.ATTRIBUTION_FIELDS or not value: + continue + if field_name in seen: + continue + # `credit_line` is composed from the others; proposing one would put a sentence + # in front of the user that no component field supports. + if field_name == "credit_line": + continue + if (field_name, value.lower()) in declined: + continue + existing = getattr(asset, field_name, None) + if isinstance(existing, str) and existing.strip(): + continue + if existing is not None and not isinstance(existing, str): + continue + + seen.add(field_name) + session.add( + Suggestion( + user_id=asset.user_id, + asset_id=asset.id, + kind=KIND_ATTRIBUTION, + value=json.dumps( + {"field": field_name, "value": value, "evidence": evidence}, + sort_keys=True, + ), + model=model, + ) + ) + created += 1 + + session.commit() + return created diff --git a/backend/tests/fixtures/attributed_audio.mp3 b/backend/tests/fixtures/attributed_audio.mp3 new file mode 100644 index 0000000..5b62de6 Binary files /dev/null and b/backend/tests/fixtures/attributed_audio.mp3 differ diff --git a/backend/tests/fixtures/attributed_document.docx b/backend/tests/fixtures/attributed_document.docx new file mode 100644 index 0000000..adaccee Binary files /dev/null and b/backend/tests/fixtures/attributed_document.docx differ diff --git a/backend/tests/fixtures/attributed_document.pdf b/backend/tests/fixtures/attributed_document.pdf new file mode 100644 index 0000000..aa59a21 Binary files /dev/null and b/backend/tests/fixtures/attributed_document.pdf differ diff --git a/backend/tests/fixtures/attributed_image.jpg b/backend/tests/fixtures/attributed_image.jpg new file mode 100644 index 0000000..00ac534 Binary files /dev/null and b/backend/tests/fixtures/attributed_image.jpg differ diff --git a/backend/tests/fixtures/attributed_video.mp4 b/backend/tests/fixtures/attributed_video.mp4 new file mode 100644 index 0000000..04b8b73 Binary files /dev/null and b/backend/tests/fixtures/attributed_video.mp4 differ diff --git a/backend/tests/fixtures/sample_document.docx b/backend/tests/fixtures/sample_document.docx index 101ac24..1625c86 100644 Binary files a/backend/tests/fixtures/sample_document.docx and b/backend/tests/fixtures/sample_document.docx differ diff --git a/backend/tests/fixtures/sample_document.pptx b/backend/tests/fixtures/sample_document.pptx index c9228b7..676badf 100644 Binary files a/backend/tests/fixtures/sample_document.pptx and b/backend/tests/fixtures/sample_document.pptx differ diff --git a/backend/tests/fixtures/sample_document.xlsx b/backend/tests/fixtures/sample_document.xlsx index 2384477..e53043e 100644 Binary files a/backend/tests/fixtures/sample_document.xlsx and b/backend/tests/fixtures/sample_document.xlsx differ diff --git a/backend/tests/make_fixtures.py b/backend/tests/make_fixtures.py index 2f82a5b..cd55a70 100644 --- a/backend/tests/make_fixtures.py +++ b/backend/tests/make_fixtures.py @@ -73,10 +73,121 @@ def main() -> None: _write_minimal_pdf(FIXTURES / "sample_document.pdf") _write_office_documents() + _write_attributed_fixtures() print("wrote:", ", ".join(sorted(p.name for p in FIXTURES.iterdir()))) +def _write_attributed_fixtures() -> None: + """Files that carry real embedded attribution, for M10's harvester. + + Separate from the `sample_*` set on purpose: those deliberately carry *no* + attribution, which is what proves the harvester writes nothing when there is nothing + to read. A single set carrying metadata could not test both halves. + + Every one of these uses the same cast — Jane Doe at the BBC, Panorama, 2019-03-15 — + so one assertion shape covers every format and a mismatch is obvious on sight. + """ + from PIL import Image + + # Video: MP4 tag block. `title` is set deliberately and must NOT be harvested — in a + # media container it names this file rather than a containing work, and mapping it + # would rename half the library on upload. `album` is what stands in for the work. + run([ + "ffmpeg", "-y", "-nostdin", + "-f", "lavfi", "-i", "testsrc=size=160x120:rate=10:duration=1", + "-c:v", "libx264", "-pix_fmt", "yuv420p", + "-metadata", "artist=Jane Doe", + "-metadata", "album=Panorama", + # MP4 has no publisher atom, so ffmpeg drops this one — deliberately left in + # to document that, and why the video fixture has no publisher while the MP3 + # below (ID3 has TPUB) does. + "-metadata", "publisher=BBC", + "-metadata", "copyright=(C) 2019 BBC", + "-metadata", "date=2019-03-15", + "-metadata", "title=Encoder boilerplate that must not be harvested", + "-metadata", "comment=https://example.org/panorama", + str(FIXTURES / "attributed_video.mp4"), + ]) + + # Audio: ID3. A bare year, which is the common case and the reason + # `published_date` is a partial-date string rather than a DateTime. + run([ + "ffmpeg", "-y", "-nostdin", + "-f", "lavfi", "-i", "sine=frequency=440:duration=1", + "-c:a", "libmp3lame", + "-metadata", "artist=Jane Doe", + "-metadata", "album=Panorama", + "-metadata", "publisher=BBC", + "-metadata", "date=2019", + str(FIXTURES / "attributed_audio.mp3"), + ]) + + # Image: EXIF. DateTimeOriginal goes in the Exif sub-IFD (0x8769) where a real + # camera puts it, not in IFD0 — a fixture that put it at the top level would let a + # harvester that only looks there pass while failing on every actual photograph. + image = Image.new("RGB", (320, 240), (60, 120, 180)) + exif = image.getexif() + exif[0x013B] = "Jane Doe" + exif[0x8298] = "(C) 2019 BBC" + # Assigned back, not just mutated: Pillow serialises the sub-IFD from the value + # stored under 0x8769, so mutating the dict get_ifd() returns is silently dropped on + # save. That mistake produces a fixture with no DateTimeOriginal at all, which a + # harvester bug would then "pass" against. + sub_ifd = exif.get_ifd(0x8769) + sub_ifd[0x9003] = "2019:03:15 10:11:12" + exif[0x8769] = sub_ifd + image.save(FIXTURES / "attributed_image.jpg", exif=exif, quality=90) + + _write_attributed_pdf(FIXTURES / "attributed_document.pdf") + + import docx + + document = docx.Document() + document.add_paragraph("Gecko Asset Manager") + properties = document.core_properties + properties.author = "Jane Doe" + properties.title = "Panorama" + properties.created = __import__("datetime").datetime(2019, 3, 15, 10, 11, 12) + document.save(FIXTURES / "attributed_document.docx") + + +def _write_attributed_pdf(target: Path) -> None: + """A one-page PDF carrying an Info dictionary. + + Same hand-built approach as `_write_minimal_pdf`, plus the /Info trailer entry that + holds Author, Title and CreationDate. PDF dates use the D:YYYYMMDDHHmmSS form, which + the harvester's date normaliser has to reduce like every other format's. + """ + objects = [ + b"<< /Type /Catalog /Pages 2 0 R >>", + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>", + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] " + b"/Resources << /Font << /F1 5 0 R >> >> /Contents 4 0 R >>", + b"<< /Length 62 >>\nstream\nBT /F1 18 Tf 20 100 Td (Gecko Asset Manager) Tj ET\nendstream", + b"<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>", + b"<< /Author (Jane Doe) /Title (Panorama) /CreationDate (D:20190315101112Z) >>", + ] + + out = bytearray(b"%PDF-1.4\n") + offsets = [] + for index, body in enumerate(objects, start=1): + offsets.append(len(out)) + out += f"{index} 0 obj\n".encode() + body + b"\nendobj\n" + + xref_at = len(out) + out += f"xref\n0 {len(objects) + 1}\n".encode() + out += b"0000000000 65535 f \n" + for offset in offsets: + out += f"{offset:010d} 00000 n \n".encode() + out += ( + f"trailer\n<< /Size {len(objects) + 1} /Root 1 0 R /Info {len(objects)} 0 R >>\n" + f"startxref\n{xref_at}\n".encode() + + b"%%EOF\n" + ) + target.write_bytes(bytes(out)) + + def _write_office_documents() -> None: """The Office and plain-text fixtures `extract_text` reads. diff --git a/backend/tests/test_attribute.py b/backend/tests/test_attribute.py new file mode 100644 index 0000000..2b7e16b --- /dev/null +++ b/backend/tests/test_attribute.py @@ -0,0 +1,390 @@ +"""Proposing where an asset came from, and writing none of it (M10). + +The cases that matter most are the ones proving the grounding rule holds: a field the +model cannot point to must not reach a suggestion, and a suggestion must not reach the +asset until somebody accepts it. A fabricated citation is worse than a blank one, and +every test here is about the difference. +""" + +import json + +import httpx +import pytest +from sqlmodel import select + +from app.enrichment import attribute +from app.models.asset import Asset +from app.models.job import EnrichmentJob, KIND_ATTRIBUTE +from app.models.suggestion import ( + KIND_ATTRIBUTION, + STATUS_PENDING, + STATUS_REJECTED, + Suggestion, +) +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, configure_provider, add_transcript + +TEST_USER = "user-under-test" + +REPLY = { + "fields": [ + { + "field": "publisher", + "value": "BBC Two", + "evidence": "the lower third reads BBC TWO", + }, + { + "field": "source_title", + "value": "Panorama", + "evidence": "the presenter says 'welcome to Panorama'", + }, + ] +} + + +@pytest.fixture(name="upstream") +def upstream_fixture(monkeypatch): + state = {"calls": [], "reply": json.dumps(REPLY)} + + def fake_post_json(url, *, headers=None, json_body, timeout, label, **kwargs): + state["calls"].append({"url": url, "body": json_body}) + return httpx.Response( + 200, + json={ + "content": [{"type": "text", "text": state["reply"]}], + "usage": {"input_tokens": 500, "output_tokens": 30}, + }, + ) + + monkeypatch.setattr(_upstream, "post_json", fake_post_json) + return state + + +def sent_prompt(state) -> str: + content = state["calls"][0]["body"]["messages"][0]["content"] + return "\n".join(b["text"] for b in content if b.get("type") == "text") + + +def pending(session, asset_id): + return suggestion_service.pending_for(session, asset_id) + + +# ─── the grounding rule ────────────────────────────────────────────────────── + + +def test_a_grounded_reply_parses(): + found = attribute.parse_reply(json.dumps(REPLY)) + + assert [f["field"] for f in found] == ["publisher", "source_title"] + assert found[0]["evidence"] == "the lower third reads BBC TWO" + + +def test_a_field_with_no_evidence_is_dropped(): + """The rule this whole job is shaped around. A model that returns a publisher with + no evidence has guessed, whatever the prompt asked for.""" + found = attribute.parse_reply( + json.dumps({"fields": [{"field": "publisher", "value": "BBC Two"}]}) + ) + assert found == [] + + +def test_an_empty_evidence_string_is_also_ungrounded(): + found = attribute.parse_reply( + json.dumps({"fields": [{"field": "publisher", "value": "BBC", "evidence": " "}]}) + ) + assert found == [] + + +def test_grounded_and_ungrounded_fields_in_one_reply_are_separated(): + found = attribute.parse_reply( + json.dumps( + { + "fields": [ + {"field": "publisher", "value": "BBC", "evidence": "the logo"}, + {"field": "creator", "value": "Probably someone famous"}, + ] + } + ) + ) + assert [f["field"] for f in found] == ["publisher"] + + +def test_an_empty_field_list_is_a_valid_answer(): + """Most material does not say where it came from, and saying so is the point.""" + assert attribute.parse_reply('{"fields": []}') == [] + + +def test_an_unknown_field_name_is_ignored(): + found = attribute.parse_reply( + json.dumps({"fields": [{"field": "vibe", "value": "ominous", "evidence": "the music"}]}) + ) + assert found == [] + + +def test_credit_line_cannot_be_proposed(): + """It is composed from the others; a proposed sentence would assert more than the + component fields support.""" + found = attribute.parse_reply( + json.dumps( + {"fields": [{"field": "credit_line", "value": "Courtesy BBC", "evidence": "x"}]} + ) + ) + assert found == [] + + +def test_a_loose_date_is_normalised_to_the_stored_format(): + found = attribute.parse_reply( + json.dumps( + { + "fields": [ + { + "field": "published_date", + "value": "2019-03-15T00:00:00Z", + "evidence": "the title card", + } + ] + } + ) + ) + assert found[0]["value"] == "2019-03-15" + + +def test_an_unreadable_date_is_dropped_rather_than_stored(): + """The column has a format and the API refuses anything else, so a suggestion the + user could never accept is worse than no suggestion.""" + found = attribute.parse_reply( + json.dumps( + { + "fields": [ + {"field": "published_date", "value": "sometime in the 90s", "evidence": "x"} + ] + } + ) + ) + assert found == [] + + +def test_a_fenced_reply_still_parses(): + found = attribute.parse_reply("```json\n" + json.dumps(REPLY) + "\n```") + assert len(found) == 2 + + +def test_a_reply_with_a_preamble_still_parses(): + found = attribute.parse_reply("Here you go:\n" + json.dumps(REPLY) + "\nHope that helps!") + assert len(found) == 2 + + +def test_an_empty_reply_is_an_error(): + with pytest.raises(ProviderError): + attribute.parse_reply("") + + +def test_unusable_json_is_an_error(): + with pytest.raises(ProviderError): + attribute.parse_reply("I could not tell you.") + + +# ─── the prompt ────────────────────────────────────────────────────────────── + + +def test_the_prompt_forbids_guessing(library, session, upstream): + configure_provider(session) + asset = _upload_image(library) + add_transcript(session, asset["id"], ["Welcome to Panorama."]) + + attribute.run(session, session.get(Asset, asset["id"]), lambda *a, **k: None) + + system = upstream["calls"][0]["body"]["system"] + assert "evidence" in system + assert "Do not infer" in system + + +def test_already_recorded_fields_are_named_so_they_are_not_re_proposed( + library, session, upstream +): + configure_provider(session) + asset = _upload_image(library) + library.patch(f"/api/assets/{asset['id']}", json={"publisher": "BBC"}) + add_transcript(session, asset["id"], ["Welcome."]) + + attribute.run(session, session.get(Asset, asset["id"]), lambda *a, **k: None) + + assert "Already recorded" in sent_prompt(upstream) + + +# ─── nothing is written ────────────────────────────────────────────────────── + + +def test_a_run_writes_no_attribution_onto_the_asset(library, session, upstream): + """The whole shape of this job. A citation only lands when a person says so.""" + configure_provider(session) + asset = _upload_image(library) + add_transcript(session, asset["id"], ["Welcome to Panorama."]) + + attribute.run(session, session.get(Asset, asset["id"]), lambda *a, **k: None) + + stored = session.get(Asset, asset["id"]) + assert stored.publisher is None + assert stored.source_title is None + assert len(pending(session, asset["id"])) == 2 + + +def test_a_suggestion_carries_its_evidence(library, session, upstream): + configure_provider(session) + asset = _upload_image(library) + add_transcript(session, asset["id"], ["Welcome."]) + + attribute.run(session, session.get(Asset, asset["id"]), lambda *a, **k: None) + + rows = library.get(f"/api/assets/{asset['id']}/suggestions").json()["data"] + publisher = next(r for r in rows if r["field"] == "publisher") + assert publisher["proposed_value"] == "BBC Two" + assert publisher["evidence"] == "the lower third reads BBC TWO" + + +def test_accepting_writes_the_field_as_human(library, session, upstream): + configure_provider(session) + asset = _upload_image(library) + add_transcript(session, asset["id"], ["Welcome."]) + attribute.run(session, session.get(Asset, asset["id"]), lambda *a, **k: None) + + row = next(r for r in pending(session, asset["id"]) if "publisher" in r.value) + library.post(f"/api/assets/{asset['id']}/suggestions/{row.id}/accept") + + refreshed = library.get(f"/api/assets/{asset['id']}").json()["data"] + assert refreshed["publisher"] == "BBC Two" + + stored = session.get(Asset, asset["id"]) + # "human", because the user read the evidence and chose it — the same reasoning the + # accepted title already follows. + assert json.loads(stored.field_provenance)["publisher"] == "human" + + +def test_rejecting_one_leaves_the_other(library, session, upstream): + configure_provider(session) + asset = _upload_image(library) + add_transcript(session, asset["id"], ["Welcome."]) + attribute.run(session, session.get(Asset, asset["id"]), lambda *a, **k: None) + + row = next(r for r in pending(session, asset["id"]) if "publisher" in r.value) + library.post(f"/api/assets/{asset['id']}/suggestions/{row.id}/reject") + + remaining = pending(session, asset["id"]) + assert len(remaining) == 1 + assert "source_title" in remaining[0].value + + +def test_a_declined_field_is_not_proposed_again(library, session, upstream): + """Being asked twice about something you said no to is how a suggestion feature + becomes noise.""" + configure_provider(session) + asset = _upload_image(library) + add_transcript(session, asset["id"], ["Welcome."]) + attribute.run(session, session.get(Asset, asset["id"]), lambda *a, **k: None) + + row = next(r for r in pending(session, asset["id"]) if "publisher" in r.value) + suggestion_service.reject(session, row) + + attribute.run(session, session.get(Asset, asset["id"]), lambda *a, **k: None) + + assert all("publisher" not in r.value for r in pending(session, asset["id"])) + + +def test_a_field_already_filled_in_is_not_proposed_over(library, session, upstream): + """Accepting writes through the human path, so allowing a proposal over an answer + would make "accept" a way to quietly overwrite one.""" + configure_provider(session) + asset = _upload_image(library) + library.patch(f"/api/assets/{asset['id']}", json={"publisher": "Channel 4"}) + add_transcript(session, asset["id"], ["Welcome."]) + + attribute.run(session, session.get(Asset, asset["id"]), lambda *a, **k: None) + + assert all("publisher" not in r.value for r in pending(session, asset["id"])) + + +def test_attribution_suggestions_do_not_clear_tag_suggestions(library, session, upstream): + """The two jobs share a table; clearing every pending row would throw away + suggestions the user has not seen from a run this one knows nothing about.""" + configure_provider(session) + asset = _upload_image(library) + add_transcript(session, asset["id"], ["Welcome."]) + + session.add( + Suggestion(user_id=TEST_USER, asset_id=asset["id"], kind="tag", value="nato", model="m") + ) + session.commit() + + attribute.run(session, session.get(Asset, asset["id"]), lambda *a, **k: None) + + kinds = {r.kind for r in pending(session, asset["id"])} + assert kinds == {"tag", KIND_ATTRIBUTION} + + +def test_a_run_that_grounds_nothing_says_so(library, session, upstream): + configure_provider(session) + asset = _upload_image(library) + add_transcript(session, asset["id"], ["Welcome."]) + upstream["reply"] = '{"fields": []}' + + detail = attribute.run(session, session.get(Asset, asset["id"]), lambda *a, **k: None) + + assert detail == "Nothing it could point to" + assert pending(session, asset["id"]) == [] + + +# ─── the endpoint ──────────────────────────────────────────────────────────── + + +def test_the_endpoint_queues_a_job(library, session, upstream): + configure_provider(session) + asset = _upload_image(library) + add_transcript(session, asset["id"], ["Welcome."]) + + response = library.post(f"/api/assets/{asset['id']}/attribute") + + assert response.status_code == 202 + assert response.json()["data"]["action"] == KIND_ATTRIBUTE + + +def test_a_second_run_while_one_is_going_is_a_409(library, session, upstream): + configure_provider(session) + asset = _upload_image(library) + add_transcript(session, asset["id"], ["Welcome."]) + + library.post(f"/api/assets/{asset['id']}/attribute") + assert library.post(f"/api/assets/{asset['id']}/attribute").status_code == 409 + + +def test_without_a_provider_it_refuses_rather_than_queueing(library, session): + asset = _upload_image(library) + add_transcript(session, asset["id"], ["Welcome."]) + + response = library.post(f"/api/assets/{asset['id']}/attribute") + + assert response.status_code == 400 + assert session.exec(select(EnrichmentJob)).all() == [] + + +# ─── decoding ──────────────────────────────────────────────────────────────── + + +def test_a_malformed_payload_is_skipped_rather_than_breaking_the_panel(): + row = Suggestion( + user_id="u", asset_id="a", kind=KIND_ATTRIBUTION, value="not json", model="m" + ) + assert suggestion_service.decode_attribution(row) == {} + + +def test_a_payload_naming_an_unknown_field_is_skipped(): + row = Suggestion( + user_id="u", + asset_id="a", + kind=KIND_ATTRIBUTION, + value=json.dumps({"field": "vibe", "value": "ominous"}), + model="m", + ) + assert suggestion_service.decode_attribution(row) == {} diff --git a/backend/tests/test_attribution.py b/backend/tests/test_attribution.py new file mode 100644 index 0000000..c6e1c81 --- /dev/null +++ b/backend/tests/test_attribution.py @@ -0,0 +1,228 @@ +"""Composition and inheritance rules for attribution (M10). + +`app/attribution.py` is pure on purpose — no session, no I/O — so every rule in it can +be driven directly here with plain objects, rather than through an upload and a clip. +""" + +from datetime import datetime +from pathlib import Path +from types import SimpleNamespace + +from app.attribution import ( + ATTRIBUTION_FIELDS, + ATTRIBUTION_TEXT_FIELDS, + compose_credit, + index_text, + resolve, +) + +BACKEND_ROOT = Path(__file__).resolve().parents[1] + + +def attributed(**fields): + """An object shaped like an Asset for the fields this module reads.""" + values = {name: None for name in ATTRIBUTION_FIELDS} + values.update(fields) + return SimpleNamespace(**values) + + +# ─── composition ───────────────────────────────────────────────────────────── + + +def test_a_full_credit_reads_in_order(): + asset = attributed( + creator="Jane Doe", + source_title="Panorama", + publisher="BBC", + published_date="2019-03", + license="CC BY 4.0", + ) + assert compose_credit(asset) == "Jane Doe — Panorama — BBC — 2019-03 — CC BY 4.0" + + +def test_a_single_field_gets_no_separator(): + """The failure mode a naive join produces: " — BBC" or "BBC — ".""" + assert compose_credit(attributed(publisher="BBC")) == "BBC" + + +def test_missing_fields_are_skipped_not_padded(): + asset = attributed(creator="Jane Doe", publisher="BBC") + assert compose_credit(asset) == "Jane Doe — BBC" + + +def test_nothing_recorded_composes_to_empty_string(): + """Not " — — — — ", and not None: the UI renders this straight into a field.""" + assert compose_credit(attributed()) == "" + + +def test_whitespace_only_fields_count_as_missing(): + asset = attributed(creator=" ", publisher="BBC") + assert compose_credit(asset) == "BBC" + + +def test_values_are_stripped(): + assert compose_credit(attributed(publisher=" BBC ")) == "BBC" + + +def test_credit_line_is_not_part_of_the_composition(): + """It is what the composition is an alternative *to*, not an input to it.""" + asset = attributed(publisher="BBC", credit_line="Something else entirely") + assert compose_credit(asset) == "BBC" + + +# ─── the override ──────────────────────────────────────────────────────────── + + +def test_the_override_wins_over_the_composition(): + resolved = resolve(attributed(publisher="BBC", credit_line="Courtesy of the BBC")) + assert resolved.credit == "Courtesy of the BBC" + + +def test_without_an_override_the_composition_is_used(): + resolved = resolve(attributed(creator="Jane Doe", publisher="BBC")) + assert resolved.credit == "Jane Doe — BBC" + + +def test_a_blank_override_falls_back_rather_than_blanking_the_credit(): + resolved = resolve(attributed(publisher="BBC", credit_line=" ")) + assert resolved.credit == "BBC" + + +# ─── inheritance ───────────────────────────────────────────────────────────── + + +def test_a_clip_with_nothing_of_its_own_inherits_every_field(): + parent = attributed(creator="Jane Doe", publisher="BBC", source_title="Panorama") + resolved = resolve(attributed(), parent) + + assert resolved.creator == "Jane Doe" + assert resolved.publisher == "BBC" + assert set(resolved.inherited) == {"creator", "publisher", "source_title"} + + +def test_an_override_is_per_field_not_all_or_nothing(): + """The whole reason inheritance coalesces field by field.""" + parent = attributed(creator="Jane Doe", publisher="BBC", source_title="Panorama") + clip = attributed(creator="Someone Else") + resolved = resolve(clip, parent) + + assert resolved.creator == "Someone Else" + assert resolved.publisher == "BBC" + assert "creator" not in resolved.inherited + assert "publisher" in resolved.inherited + + +def test_correcting_the_parent_changes_what_the_clip_resolves_to(): + """Resolution happens on read, so there is no copy anywhere to go stale. + + This is the property that made copy-on-create the wrong design: a correction has to + reach the clips already cut from it. + """ + parent = attributed(publisher="BBC Two") + clip = attributed() + assert resolve(clip, parent).publisher == "BBC Two" + + parent.publisher = "BBC Four" + assert resolve(clip, parent).publisher == "BBC Four" + + +def test_a_blank_field_on_the_clip_still_inherits(): + parent = attributed(publisher="BBC") + assert resolve(attributed(publisher=" "), parent).publisher == "BBC" + + +def test_no_parent_resolves_to_the_asset_alone(): + resolved = resolve(attributed(publisher="BBC")) + assert resolved.publisher == "BBC" + assert resolved.inherited == () + + +def test_a_parent_with_nothing_recorded_inherits_nothing(): + resolved = resolve(attributed(), attributed()) + assert resolved.inherited == () + assert resolved.is_empty + + +def test_retrieved_at_inherits_as_a_datetime(): + """The one non-string field: blankness is not emptiness for a datetime.""" + when = datetime(2026, 1, 1, 12, 0, 0) + resolved = resolve(attributed(), attributed(retrieved_at=when)) + assert resolved.retrieved_at == when + assert "retrieved_at" in resolved.inherited + + +# ─── is_empty, which is what the unattributed filter means ─────────────────── + + +def test_is_empty_is_true_only_when_nothing_at_all_is_recorded(): + assert resolve(attributed()).is_empty + assert not resolve(attributed(source_url="https://example.org")).is_empty + assert not resolve(attributed(retrieved_at=datetime(2026, 1, 1))).is_empty + + +def test_an_inheriting_clip_is_not_empty(): + """It has attribution — it just did not type it itself.""" + assert not resolve(attributed(), attributed(publisher="BBC")).is_empty + + +# ─── the search text ───────────────────────────────────────────────────────── + + +def test_index_text_carries_the_who_and_the_where(): + resolved = resolve( + attributed( + creator="Jane Doe", + publisher="BBC", + source_title="Panorama", + license="CC BY 4.0", + source_url="https://example.org/x", + ) + ) + text = index_text(resolved) + for term in ("Jane Doe", "BBC", "Panorama", "CC BY 4.0", "https://example.org/x"): + assert term in text + + +def test_index_text_leaves_dates_out(): + """Both are served exactly by the date-range filters; a bare year in a text index + mostly collides with titles instead of helping.""" + resolved = resolve( + attributed(published_date="1994", retrieved_at=datetime(2026, 1, 1), publisher="BBC") + ) + assert index_text(resolved) == "BBC" + + +def test_index_text_of_nothing_is_empty(): + assert index_text(resolve(attributed())) == "" + + +def test_a_clip_is_indexed_under_what_it_inherited(): + """Otherwise "everything from the BBC" is quietly missing every clip.""" + resolved = resolve(attributed(), attributed(publisher="BBC")) + assert "BBC" in index_text(resolved) + + +# ─── the two copies of the field list ──────────────────────────────────────── + + +def test_the_migration_indexes_the_same_fields_this_module_does(): + """`9c2e08b4a1f7` carries a literal copy of the attribution_text expression. + + That duplication is deliberate — a migration has to describe the schema as it was + when it was written, so it cannot import this module. This is what stops the two + drifting: if a field is added here and not there, an existing library is repopulated + without it and stays unsearchable by that field until every asset is edited again. + """ + migration = ( + BACKEND_ROOT + / "alembic" + / "versions" + / "20260920_0731_rebuild_asset_fts_with_attribution.py" + ).read_text() + expression = migration.split("TRIM(")[1].split("FROM asset")[0] + + for name in ATTRIBUTION_TEXT_FIELDS: + assert f"a.{name}" in expression, f"the migration does not index {name}" + + for name in ("published_date", "retrieved_at"): + assert f"a.{name}" not in expression, f"the migration indexes {name}, this module does not" diff --git a/backend/tests/test_attribution_api.py b/backend/tests/test_attribution_api.py new file mode 100644 index 0000000..e805b72 --- /dev/null +++ b/backend/tests/test_attribution_api.py @@ -0,0 +1,388 @@ +"""Attribution over HTTP: editing, inheritance, filtering and search (M10).""" + +from pathlib import Path + +from sqlmodel import select + +from app.models.asset import Asset + +FIXTURES = Path(__file__).parent / "fixtures" + + +def _upload(client, *names: str): + files = [ + ("files", (name, (FIXTURES / name).read_bytes(), "application/octet-stream")) + for name in names + ] + return client.post("/api/assets", files=files) + + +def _upload_one(client, name: str = "sample_video.mp4") -> dict: + return _upload(client, name).json()["created"][0] + + +# ─── editing ───────────────────────────────────────────────────────────────── + + +def test_attribution_is_editable_by_hand(library): + asset = _upload_one(library) + + response = library.patch( + f"/api/assets/{asset['id']}", + json={ + "creator": "Jane Doe", + "publisher": "BBC", + "source_title": "Panorama", + "published_date": "2019-03", + "license": "CC BY 4.0", + "source_url": "https://example.org/panorama", + }, + ) + assert response.status_code == 200 + + updated = response.json()["data"] + assert updated["publisher"] == "BBC" + assert updated["credit"] == "Jane Doe — Panorama — BBC — 2019-03 — CC BY 4.0" + + +def test_a_typed_credit_line_overrides_the_composition(library): + asset = _upload_one(library) + response = library.patch( + f"/api/assets/{asset['id']}", + json={"publisher": "BBC", "credit_line": "Courtesy of the BBC"}, + ) + assert response.json()["data"]["credit"] == "Courtesy of the BBC" + + +def test_correcting_a_field_recomposes_the_credit(library): + """The reason the composition is not stored.""" + asset = _upload_one(library) + library.patch(f"/api/assets/{asset['id']}", json={"publisher": "BBC Two"}) + assert library.get(f"/api/assets/{asset['id']}").json()["data"]["credit"] == "BBC Two" + + library.patch(f"/api/assets/{asset['id']}", json={"publisher": "BBC Four"}) + assert library.get(f"/api/assets/{asset['id']}").json()["data"]["credit"] == "BBC Four" + + +def test_a_partial_published_date_is_accepted(library): + asset = _upload_one(library) + for value in ("1994", "2019-03", "2019-03-15"): + response = library.patch(f"/api/assets/{asset['id']}", json={"published_date": value}) + assert response.status_code == 200, value + assert response.json()["data"]["published_date"] == value + + +def test_a_malformed_published_date_is_refused(library): + """The string column is what allows a partial date; the validator is what stops it + becoming free text, which would break the lexicographic range filters.""" + asset = _upload_one(library) + for value in ("summer 1994", "15/03/2019", "2019-13", "2019-3"): + response = library.patch(f"/api/assets/{asset['id']}", json={"published_date": value}) + assert response.status_code == 422, value + + +# ─── inheritance ───────────────────────────────────────────────────────────── + + +def _clip_of(library, parent_id: str) -> dict: + response = library.post( + f"/api/assets/{parent_id}/clips", json={"in_point": 0.2, "out_point": 0.8} + ) + assert response.status_code == 201, response.text + return response.json()["data"] + + +def test_a_clip_inherits_its_parents_attribution(library): + parent = _upload_one(library) + library.patch( + f"/api/assets/{parent['id']}", + json={"creator": "Jane Doe", "publisher": "BBC", "source_title": "Panorama"}, + ) + + clip = _clip_of(library, parent["id"]) + assert clip["publisher"] == "BBC" + assert clip["credit"] == "Jane Doe — Panorama — BBC" + assert set(clip["attribution_inherited"]) == {"creator", "publisher", "source_title"} + + +def test_a_clip_can_override_one_field_and_keep_the_rest(library): + parent = _upload_one(library) + library.patch( + f"/api/assets/{parent['id']}", + json={"creator": "Jane Doe", "publisher": "BBC"}, + ) + clip = _clip_of(library, parent["id"]) + + updated = library.patch( + f"/api/assets/{clip['id']}", json={"creator": "A different interviewee"} + ).json()["data"] + + assert updated["creator"] == "A different interviewee" + assert updated["publisher"] == "BBC" + assert updated["attribution_inherited"] == ["publisher"] + + +def test_correcting_the_parent_reaches_every_clip_of_it(library): + """The property that made copy-on-create the wrong design.""" + parent = _upload_one(library) + library.patch(f"/api/assets/{parent['id']}", json={"publisher": "BBC Two"}) + clip = _clip_of(library, parent["id"]) + assert clip["publisher"] == "BBC Two" + + library.patch(f"/api/assets/{parent['id']}", json={"publisher": "BBC Four"}) + + refreshed = library.get(f"/api/assets/{clip['id']}").json()["data"] + assert refreshed["publisher"] == "BBC Four" + + +def test_an_unattributed_parent_leaves_the_clip_blank(library): + parent = _upload_one(library) + clip = _clip_of(library, parent["id"]) + assert clip["credit"] == "" + assert clip["attribution_inherited"] == [] + + +# ─── filters ───────────────────────────────────────────────────────────────── + + +def test_filtering_by_publisher(library): + first = _upload_one(library, "sample_video.mp4") + second = _upload_one(library, "sample_image.jpg") + library.patch(f"/api/assets/{first['id']}", json={"publisher": "BBC"}) + library.patch(f"/api/assets/{second['id']}", json={"publisher": "Channel 4"}) + + body = library.get("/api/assets", params={"publisher": "BBC"}).json() + assert [row["id"] for row in body["data"]] == [first["id"]] + assert body["total"] == 1 + + +def test_filtering_by_publisher_also_returns_the_clips_that_inherit_it(library): + """A filter that only looked at the row would answer "everything from the BBC" with + the documentary and none of the clips cut from it.""" + parent = _upload_one(library) + library.patch(f"/api/assets/{parent['id']}", json={"publisher": "BBC"}) + clip = _clip_of(library, parent["id"]) + + body = library.get("/api/assets", params={"publisher": "BBC"}).json() + assert {row["id"] for row in body["data"]} == {parent["id"], clip["id"]} + + +def test_a_clip_that_overrides_the_publisher_drops_out_of_the_parents_filter(library): + parent = _upload_one(library) + library.patch(f"/api/assets/{parent['id']}", json={"publisher": "BBC"}) + clip = _clip_of(library, parent["id"]) + library.patch(f"/api/assets/{clip['id']}", json={"publisher": "Channel 4"}) + + body = library.get("/api/assets", params={"publisher": "BBC"}).json() + assert [row["id"] for row in body["data"]] == [parent["id"]] + + +def test_filtering_by_creator_and_source_title(library): + asset = _upload_one(library) + library.patch( + f"/api/assets/{asset['id']}", json={"creator": "Jane Doe", "source_title": "Panorama"} + ) + + assert library.get("/api/assets", params={"creator": "jane"}).json()["total"] == 1 + assert library.get("/api/assets", params={"source_title": "panorama"}).json()["total"] == 1 + assert library.get("/api/assets", params={"creator": "nobody"}).json()["total"] == 0 + + +def test_published_date_ranges_compare_chronologically(library): + """Lexicographic comparison is chronological because the format is guaranteed.""" + older = _upload_one(library, "sample_video.mp4") + newer = _upload_one(library, "sample_image.jpg") + library.patch(f"/api/assets/{older['id']}", json={"published_date": "2018-12-31"}) + library.patch(f"/api/assets/{newer['id']}", json={"published_date": "2019-03-01"}) + + after = library.get("/api/assets", params={"published_after": "2019"}).json() + assert [row["id"] for row in after["data"]] == [newer["id"]] + + before = library.get("/api/assets", params={"published_before": "2019"}).json() + assert [row["id"] for row in before["data"]] == [older["id"]] + + +def test_unattributed_returns_exactly_what_is_still_missing_a_source(library): + attributed = _upload_one(library, "sample_video.mp4") + blank = _upload_one(library, "sample_image.jpg") + library.patch(f"/api/assets/{attributed['id']}", json={"publisher": "BBC"}) + + body = library.get("/api/assets", params={"unattributed": "true"}).json() + assert [row["id"] for row in body["data"]] == [blank["id"]] + + inverse = library.get("/api/assets", params={"unattributed": "false"}).json() + assert [row["id"] for row in inverse["data"]] == [attributed["id"]] + + +def test_a_clip_that_inherits_is_not_unattributed(library): + """It is not missing a source — it shows its parent's. Counting it would put rows in + the backlog that there is nothing to do about.""" + parent = _upload_one(library) + library.patch(f"/api/assets/{parent['id']}", json={"publisher": "BBC"}) + clip = _clip_of(library, parent["id"]) + + ids = {row["id"] for row in library.get("/api/assets", params={"unattributed": "true"}).json()["data"]} + assert clip["id"] not in ids + assert parent["id"] not in ids + + +def test_a_whitespace_only_value_still_counts_as_unattributed(library): + asset = _upload_one(library) + library.patch(f"/api/assets/{asset['id']}", json={"publisher": " "}) + + body = library.get("/api/assets", params={"unattributed": "true"}).json() + assert [row["id"] for row in body["data"]] == [asset["id"]] + + +# ─── search ────────────────────────────────────────────────────────────────── + + +def test_an_asset_is_findable_by_its_publisher(library): + asset = _upload_one(library) + library.patch( + f"/api/assets/{asset['id']}", json={"publisher": "BBC", "source_title": "Panorama"} + ) + + for term in ("BBC", "Panorama"): + body = library.get("/api/search", params={"q": term}).json() + assert [hit["asset"]["id"] for hit in body["data"]] == [asset["id"]], term + + +def test_a_clip_is_findable_by_the_publisher_it_inherited(library): + parent = _upload_one(library) + library.patch(f"/api/assets/{parent['id']}", json={"publisher": "BBC"}) + clip = _clip_of(library, parent["id"]) + + found = {hit["asset"]["id"] for hit in library.get("/api/search", params={"q": "BBC"}).json()["data"]} + assert clip["id"] in found + + +def test_correcting_a_parent_reindexes_its_clips(library, session): + """The keyword index stores a snapshot, so it is the one place inheritance cannot be + resolved on read — without the child re-index, a clip stays searchable under the old + publisher and invisible under the new one.""" + parent = _upload_one(library) + library.patch(f"/api/assets/{parent['id']}", json={"publisher": "BBC"}) + clip = _clip_of(library, parent["id"]) + + library.patch(f"/api/assets/{parent['id']}", json={"publisher": "Channel 4"}) + + found = {hit["asset"]["id"] for hit in library.get("/api/search", params={"q": "Channel"}).json()["data"]} + assert clip["id"] in found + + stale = {hit["asset"]["id"] for hit in library.get("/api/search", params={"q": "BBC"}).json()["data"]} + assert clip["id"] not in stale + + +def test_attribution_does_not_leak_between_users(library, session): + """Every filter above narrows within one user's library; this is the one that would + be a disclosure rather than a wrong answer.""" + asset = _upload_one(library) + library.patch(f"/api/assets/{asset['id']}", json={"publisher": "BBC"}) + + stored = session.exec(select(Asset)).first() + stored.user_id = "somebody-else" + session.add(stored) + session.commit() + + assert library.get("/api/assets", params={"publisher": "BBC"}).json()["total"] == 0 + assert library.get("/api/search", params={"q": "BBC"}).json()["data"] == [] + + +# ─── the library-wide re-harvest ───────────────────────────────────────────── + + +def _run_job(job_id, session, monkeypatch): + """Run a queued job against the test database. + + The queue builds its own session from its own engine, so it has to be pointed at + this test's — the same helper `test_embedding_backfill.py` uses for library jobs. + """ + from app.jobs import enrichment as enrichment_jobs + + monkeypatch.setattr(enrichment_jobs.queue(), "engine", session.get_bind()) + enrichment_jobs._run_job(job_id) + session.expire_all() + + +def test_harvest_queues_one_library_job(library, session): + from app.models.job import KIND_HARVEST_ATTRIBUTION, EnrichmentJob + + _upload(library, "sample_image.jpg", "sample_video.mp4") + + response = library.post("/api/assets/harvest-attribution") + + assert response.status_code == 202 + body = response.json()["data"] + assert body["action"] == KIND_HARVEST_ATTRIBUTION + assert body["asset_id"] is None + assert len(session.exec(select(EnrichmentJob)).all()) == 1 + + +def test_a_second_harvest_while_one_runs_is_a_409(library, session): + library.post("/api/assets/harvest-attribution") + assert library.post("/api/assets/harvest-attribution").status_code == 409 + + +def test_the_harvest_attributes_files_uploaded_before_it_existed(library, session, monkeypatch): + """The case this endpoint exists for: the metadata was on disk the whole time and + nothing had ever looked.""" + asset = _upload_one(library, "attributed_image.jpg") + + # Wind the row back to how it would look had it been ingested before M10. + stored = session.get(Asset, asset["id"]) + stored.creator = None + stored.license = None + stored.published_date = None + stored.field_provenance = "{}" + session.add(stored) + session.commit() + + job = library.post("/api/assets/harvest-attribution").json()["data"] + _run_job(job["id"], session, monkeypatch) + + refreshed = library.get(f"/api/assets/{asset['id']}").json()["data"] + assert refreshed["creator"] == "Jane Doe" + assert refreshed["license"] == "(C) 2019 BBC" + + +def test_the_harvest_never_overwrites_a_hand_typed_value(library, session, monkeypatch): + """Which is what makes it safe to run twice, and safe to run unasked.""" + asset = _upload_one(library, "attributed_image.jpg") + library.patch(f"/api/assets/{asset['id']}", json={"creator": "The actual photographer"}) + + job = library.post("/api/assets/harvest-attribution").json()["data"] + _run_job(job["id"], session, monkeypatch) + + refreshed = library.get(f"/api/assets/{asset['id']}").json()["data"] + assert refreshed["creator"] == "The actual photographer" + + +def test_the_harvest_reports_what_it_did(library, session, monkeypatch): + from app.models.job import EnrichmentJob + + _upload(library, "attributed_image.jpg", "sample_image.jpg") + stored = session.exec(select(Asset).where(Asset.creator.is_not(None))).first() + stored.creator = None + stored.license = None + stored.published_date = None + session.add(stored) + session.commit() + + job = library.post("/api/assets/harvest-attribution").json()["data"] + _run_job(job["id"], session, monkeypatch) + + finished = session.get(EnrichmentJob, job["id"]) + assert finished.status == "done" + assert "1 attributed of 2 scanned" in finished.detail + + +def test_the_harvest_skips_clips_which_own_no_bytes(library, session): + """A clip inherits on read; there is no file under it to read metadata from.""" + from app.enrichment.harvest_attribution import run + + parent = _upload_one(library, "attributed_video.mp4") + _clip_of(library, parent["id"]) + + result = run(session, session.get(Asset, parent["id"]).user_id, lambda *a, **k: None) + assert result.scanned == 1 diff --git a/backend/tests/test_embedded_metadata.py b/backend/tests/test_embedded_metadata.py new file mode 100644 index 0000000..70f3d83 --- /dev/null +++ b/backend/tests/test_embedded_metadata.py @@ -0,0 +1,219 @@ +"""Attribution read out of a file's own container metadata (M10). + +Run against real files rather than mocked tag dictionaries: the thing most likely to be +wrong here is what a format actually stores and where, which a stub cannot be wrong +about. `tests/make_fixtures.py` builds the `attributed_*` set; the `sample_*` set +deliberately carries no attribution, which is what proves the harvester stays silent +when there is nothing to read. +""" + +from datetime import datetime +from pathlib import Path + +import pytest +from sqlmodel import select + +from app.ingest.embedded_metadata import harvest, normalise_partial_date +from app.ingest.probe import probe +from app.models.asset import PROVENANCE_EMBEDDED, Asset + +FIXTURES = Path(__file__).parent / "fixtures" + + +def _harvest(name: str, asset_type: str) -> dict: + path = FIXTURES / name + tags = probe(path).tags if asset_type in ("video", "audio") else {} + return harvest(path, asset_type, name, probe_tags=tags) + + +def _upload(client, *names: str): + files = [ + ("files", (name, (FIXTURES / name).read_bytes(), "application/octet-stream")) + for name in names + ] + return client.post("/api/assets", files=files) + + +# ─── per format ────────────────────────────────────────────────────────────── + + +def test_mp4_tag_block_is_read(): + found = _harvest("attributed_video.mp4", "video") + assert found["creator"] == "Jane Doe" + assert found["source_title"] == "Panorama" + assert found["license"] == "(C) 2019 BBC" + assert found["published_date"] == "2019-03-15" + + +def test_id3_tags_are_read(): + found = _harvest("attributed_audio.mp3", "audio") + assert found["creator"] == "Jane Doe" + assert found["publisher"] == "BBC" + assert found["source_title"] == "Panorama" + + +def test_a_bare_year_stays_a_bare_year(): + """The case the string column exists for: ID3 routinely carries only a year, and + turning it into 2019-01-01 would invent a precision the file never claimed.""" + assert _harvest("attributed_audio.mp3", "audio")["published_date"] == "2019" + + +def test_exif_is_read_including_the_sub_ifd(): + """DateTimeOriginal lives in the Exif sub-IFD (0x8769), where every real camera puts + it — a harvester reading only IFD0 passes on hand-built files and fails on photos.""" + found = _harvest("attributed_image.jpg", "image") + assert found["creator"] == "Jane Doe" + assert found["license"] == "(C) 2019 BBC" + assert found["published_date"] == "2019-03-15" + + +def test_pdf_info_dictionary_is_read(): + found = _harvest("attributed_document.pdf", "document") + assert found["creator"] == "Jane Doe" + assert found["source_title"] == "Panorama" + # D:20190315101112Z — prefixed and separator-less, unlike every other format. + assert found["published_date"] == "2019-03-15" + + +def test_docx_core_properties_are_read(): + found = _harvest("attributed_document.docx", "document") + assert found["creator"] == "Jane Doe" + assert found["source_title"] == "Panorama" + assert found["published_date"] == "2019-03-15" + + +# ─── what must NOT be harvested ────────────────────────────────────────────── + + +def test_a_media_containers_title_is_not_harvested(): + """In a media container `title` names this file, not a containing work, and is very + often an encoder's boilerplate. The fixture sets one precisely so this can assert it + goes nowhere — mapping it would rename or mis-source half a library on upload.""" + found = _harvest("attributed_video.mp4", "video") + assert "Encoder boilerplate" not in str(found) + assert found.get("source_title") == "Panorama" + assert "name" not in found + + +def test_technical_tags_are_not_mistaken_for_attribution(): + """Every MP4 carries encoder, handler_name, major_brand and compatible_brands. A + mapping that took whatever it recognised would file "Lavf60.16.100" as a creator.""" + found = _harvest("sample_video.mp4", "video") + assert found == {} + + +def test_files_with_no_attribution_yield_nothing(): + assert _harvest("sample_image.jpg", "image") == {} + assert _harvest("sample_document.pdf", "document") == {} + assert _harvest("sample_audio.mp3", "audio") == {} + + +def test_retrieved_at_is_never_harvested(): + """When *you* fetched something is not a fact the file can know.""" + for name, kind in [ + ("attributed_video.mp4", "video"), + ("attributed_image.jpg", "image"), + ("attributed_document.pdf", "document"), + ]: + assert "retrieved_at" not in _harvest(name, kind) + + +def test_a_comment_is_only_taken_as_a_url_when_it_is_one(): + """Downloaders put the source URL in `comment`. They also put everything else there.""" + from app.ingest.embedded_metadata import _from_container_tags + + assert _from_container_tags({"comment": "https://example.org/x"})["source_url"] == ( + "https://example.org/x" + ) + assert "source_url" not in _from_container_tags({"comment": "Recorded off-air, poor audio"}) + + +def test_harvest_never_raises_on_an_unreadable_file(tmp_path): + """This runs after the row is committed; an exception here must not reach the upload.""" + broken = tmp_path / "broken.jpg" + broken.write_bytes(b"not an image") + assert harvest(broken, "image", "broken.jpg") == {} + + +def test_an_unknown_document_format_is_simply_empty(): + assert _harvest("sample_document.txt", "document") == {} + + +# ─── the date normaliser ───────────────────────────────────────────────────── + + +@pytest.mark.parametrize( + "raw,expected", + [ + ("2019", "2019"), + ("2019-03", "2019-03"), + ("2019-03-15", "2019-03-15"), + ("2019-03-15T10:00:00.000000Z", "2019-03-15"), # MP4 + ("2019:03:15 10:11:12", "2019-03-15"), # EXIF, colon separated + ("D:20190315101112Z", "2019-03-15"), # PDF + ("20190315", "2019-03-15"), # compact, no prefix + ("", None), + ("not a date", None), + (None, None), + ("2019-19-40", "2019"), # degrades to what is still trustworthy + ], +) +def test_partial_dates_normalise_without_inventing_precision(raw, expected): + assert normalise_partial_date(raw) == expected + + +def test_a_real_datetime_is_accepted(): + """Office hands back a datetime object rather than a string.""" + assert normalise_partial_date(datetime(2020, 5, 6, 12, 0)) == "2020-05-06" + + +# ─── through the upload pipeline ───────────────────────────────────────────── + + +def test_an_upload_lands_attributed(library, session): + response = _upload(library, "attributed_image.jpg") + assert response.status_code == 201 + + asset = response.json()["created"][0] + assert asset["creator"] == "Jane Doe" + assert asset["license"] == "(C) 2019 BBC" + assert asset["published_date"] == "2019-03-15" + + +def test_embedded_values_are_stamped_as_embedded_not_human(library, session): + """The distinction that lets a later pass propose over a camera-supplied name + without ever proposing over something the user typed.""" + _upload(library, "attributed_image.jpg") + + stored = session.exec(select(Asset)).first() + import json + + provenance = json.loads(stored.field_provenance) + assert provenance["creator"] == PROVENANCE_EMBEDDED + + +def test_an_upload_with_no_embedded_metadata_stays_unattributed(library, session): + response = _upload(library, "sample_image.jpg") + asset = response.json()["created"][0] + + assert asset["creator"] is None + assert asset["credit"] == "" + + +def test_a_harvest_never_overwrites_a_value_already_there(library, session): + """Filtering to empty fields is what makes the harvest safe to run unasked, and safe + to re-run over a library that has been hand-corrected.""" + from app.services import assets as asset_service + + _upload(library, "attributed_image.jpg") + stored = session.exec(select(Asset)).first() + + asset_service.apply_metadata(session, stored, {"creator": "The actual photographer"}) + asset_service.apply_embedded_attribution( + session, stored, {"creator": "Jane Doe", "publisher": "BBC"} + ) + session.refresh(stored) + + assert stored.creator == "The actual photographer" + # The empty one is still filled, so a partial correction does not block the rest. + assert stored.publisher == "BBC" diff --git a/backend/tests/test_migrations.py b/backend/tests/test_migrations.py index bcba722..27b9c84 100644 --- a/backend/tests/test_migrations.py +++ b/backend/tests/test_migrations.py @@ -213,3 +213,109 @@ def test_add_clip_columns_survives_real_foreign_key_references(alembic_config): ).fetchone() is not None conn.execute("PRAGMA foreign_keys=ON") assert conn.execute("PRAGMA foreign_key_check").fetchall() == [] + + +def test_asset_fts_rebuild_preserves_the_existing_index(alembic_config): + """`9c2e08b4a1f7` drops and recreates asset_fts, which throws away its contents. + + FTS5 has no ALTER TABLE ADD COLUMN, so adding `attribution_text` means recreating + the virtual table — and because asset_fts stores its own copy of the text rather + than using external-content mode, the recreate destroys the index for every asset + already in the library. The repopulate is what puts it back, and it is invisible to + any test that only compares columns: a migration missing it passes the drift check, + leaves the schema perfect, and silently returns nothing for every keyword search + until each asset happens to be edited again. + + So this seeds a real index entry at the revision before, and asserts it is still + searchable after — which is the only assertion that can tell the two apart. + """ + config, db_path = alembic_config + command.upgrade(config, "3f7a21c9d4e5") + + with sqlite3.connect(db_path) as conn: + conn.execute( + "INSERT INTO asset (id, user_id, name, asset_type, source, storage_key," + " size_bytes, field_provenance, upload_date, modified_date, metadata_modified_date," + " description, summary, publisher, creator)" + " VALUES ('a1', 'u', 'Giordano interview', 'video', 'local_upload', 'k1'," + " 100, '{}', '2026-01-01', '2026-01-01', '2026-01-01'," + " 'A long conversation', 'Nano weapons', 'Modern Wisdom', 'James Giordano')" + ) + conn.execute( + "INSERT INTO tag (id, user_id, name, created_at)" + " VALUES ('t1', 'u', 'neuroscience', '2026-01-01')" + ) + conn.execute( + "INSERT INTO assettag (asset_id, tag_id, created_at)" + " VALUES ('a1', 't1', '2026-01-01')" + ) + # The pre-M10 six-column shape, as the old migration created it. + conn.execute( + "INSERT INTO asset_fts (asset_id, user_id, name, description, summary, tags_text)" + " VALUES ('a1', 'u', 'Giordano interview', 'A long conversation'," + " 'Nano weapons', 'neuroscience')" + ) + conn.commit() + + command.upgrade(config, "head") + + with sqlite3.connect(db_path) as conn: + assert conn.execute("SELECT count(*) FROM asset_fts").fetchone()[0] == 1 + + # The pre-existing content still matches, which is the regression that a + # missing repopulate would cause. + for term in ("Giordano", "conversation", "neuroscience"): + hit = conn.execute( + "SELECT asset_id FROM asset_fts WHERE asset_fts MATCH ?", (term,) + ).fetchone() + assert hit is not None and hit[0] == "a1", f"lost the index entry for {term!r}" + + +def test_asset_fts_rebuild_indexes_attribution(alembic_config): + """The point of the rebuild: an asset becomes findable by who published it.""" + config, db_path = alembic_config + command.upgrade(config, "3f7a21c9d4e5") + + with sqlite3.connect(db_path) as conn: + conn.execute( + "INSERT INTO asset (id, user_id, name, asset_type, source, storage_key," + " size_bytes, field_provenance, upload_date, modified_date, metadata_modified_date," + " creator, publisher, source_title, license, source_url)" + " VALUES ('a1', 'u', 'Clip', 'video', 'local_upload', 'k1'," + " 100, '{}', '2026-01-01', '2026-01-01', '2026-01-01'," + " 'Jane Doe', 'BBC', 'Panorama', 'CC BY 4.0', 'https://example.org/x')" + ) + conn.commit() + + command.upgrade(config, "head") + + with sqlite3.connect(db_path) as conn: + for term in ("BBC", "Panorama", "Jane"): + hit = conn.execute( + "SELECT asset_id FROM asset_fts WHERE asset_fts MATCH ?", (term,) + ).fetchone() + assert hit is not None and hit[0] == "a1", f"not findable by {term!r}" + + +def test_asset_fts_downgrade_also_repopulates(alembic_config): + """A downgrade that emptied the index would be a data-shaped loss, not a schema one.""" + config, db_path = alembic_config + command.upgrade(config, "3f7a21c9d4e5") + + with sqlite3.connect(db_path) as conn: + conn.execute( + "INSERT INTO asset (id, user_id, name, asset_type, source, storage_key," + " size_bytes, field_provenance, upload_date, modified_date, metadata_modified_date)" + " VALUES ('a1', 'u', 'Giordano interview', 'video', 'local_upload', 'k1'," + " 100, '{}', '2026-01-01', '2026-01-01', '2026-01-01')" + ) + conn.commit() + + command.upgrade(config, "head") + command.downgrade(config, "3f7a21c9d4e5") + + with sqlite3.connect(db_path) as conn: + hit = conn.execute( + "SELECT asset_id FROM asset_fts WHERE asset_fts MATCH 'Giordano'" + ).fetchone() + assert hit is not None and hit[0] == "a1" diff --git a/docs/m10-attribution.md b/docs/m10-attribution.md index dd5b0bc..e9215ee 100644 --- a/docs/m10-attribution.md +++ b/docs/m10-attribution.md @@ -1,6 +1,9 @@ # M10 — Attribution -**Status:** specified, not built. This document is the spec; no code exists yet. +**Status:** built. The spec below held with three changes, each recorded in place: +a library-wide re-harvest was added (the files already uploaded carry their metadata on +disk and nothing had ever looked), a bulk attribution pass over a selection was built +rather than deferred, and `rights_notes` stayed out as decided. Numbered M10 but landing **before** M8 and M9. Out of numeric order on purpose: M8 needs a fal.ai account and M9 needs GVC to exist, while this needs neither and is the one piece @@ -291,13 +294,37 @@ instance of it. - **No per-clip timestamped citation format.** A clip can override `credit_line` freehand; a structured "at 04:12" convention can wait until GVC shows what it needs. -## Open, and genuinely undecided - -Whether a bulk attribution pass over a selection is worth building, as M6 did for -enrichment. It probably is for a batch imported from one source in one sitting — twenty -screenshots from the same programme share every field. Left out of the first pass because -the `unattributed` filter plus the existing `SelectionBar` may make it a small addition -rather than a design. +## ~~Open, and genuinely undecided~~ Resolved during the build + +Whether a bulk attribution pass over a selection was worth building. **Built.** It turned +out to be one entry in `enrichment/bulk.py::ACTIONS` and one button in `SelectionBar`, +because the machinery M6 put in place already covered it — well under the cost of the +design discussion the question implied. It writes nothing either way, since every result +is a suggestion, so it carries none of the risk that keeps transcription out of that list. + +## What the build added beyond the spec + +- **A library-wide re-harvest** (`POST /api/assets/harvest-attribution`). Ingest handles + new uploads, but everything uploaded before M10 still has its EXIF and ID3 on disk with + nothing having looked. Safe to run repeatedly, because the harvest only fills blanks. +- **Child re-indexing on an attribution change.** Inheritance resolves on read everywhere + except the keyword index, which by nature stores a snapshot. Without this, correcting a + parent's publisher left every clip of it indexed under the old one — findable by a value + no longer shown anywhere. + +## Two bugs real files caught that a stubbed tag dictionary would not have + +Both would have passed against mocked metadata, and both are why the fixtures are real +files built by `tests/make_fixtures.py`: + +- **EXIF `DateTimeOriginal` lives in the Exif sub-IFD (0x8769), not IFD0.** A harvester + reading only the top level works on hand-built fixtures and fails on every actual + photograph. The fixture generator carries the matching trap: Pillow serialises the + sub-IFD from the value stored under 0x8769, so mutating the dict `get_ifd()` returns is + silently dropped on save — which produces a fixture with no date at all for a broken + reader to "pass" against. +- **PDF writes its date as `D:20190315101112Z`** — prefixed and separator-less, unlike + every other format, and unmatched by a normaliser written against the rest. --- diff --git a/docs/plan-of-attack.md b/docs/plan-of-attack.md index b9a7701..f5d6318 100644 --- a/docs/plan-of-attack.md +++ b/docs/plan-of-attack.md @@ -329,7 +329,7 @@ it was written in does not survive the session. | M3 Tagging | Merged — [#7](https://github.com/davior/gam/pull/7) (fast-forwarded, so no merge commit) | | M6 AI enrichment | **Complete, all 8 steps** — [#14](https://github.com/davior/gam/pull/14) `AIProvider`, its migration, `/api/providers` CRUD and the settings panel; [#15](https://github.com/davior/gam/pull/15) the three protocol clients and the retry/backoff layer; [#16](https://github.com/davior/gam/pull/16) the source-material spec and `summarize`; [#17](https://github.com/davior/gam/pull/17) `autotag`, the suggestion model, and generated titles; [#18](https://github.com/davior/gam/pull/18) `describe`; [#19](https://github.com/davior/gam/pull/19) `UsageEvent`, the pricing table and the cost readout; [#20](https://github.com/davior/gam/pull/20) bulk enrichment over a selection, which also picked up the `SelectionBar` embed deferred from M5. Two gaps M6 did **not** close are recorded in [`m6-ai-enrichment.md`](m6-ai-enrichment.md) and below: `extract_text`, since built, and attribution, still undesigned. | | M7 Clips & sub-videos | **Complete** — [#26](https://github.com/davior/gam/pull/26). Non-destructive clips, ffmpeg sub-video extraction (fast stream-copy with an automatic re-encode fallback), and a parent-delete guard with a one-click promote path. Two bugs caught before shipping — a promoted clip almost kept its parent-relative `in_point`/`out_point`, and the delete guard's own state was briefly getting wiped by a store-driven remount — are recorded in [`m7-clips-and-subvideos.md`](m7-clips-and-subvideos.md), along with why the milestone's actual file layout diverges from this document's own architecture sketch. | -| M10 Attribution | **Specified, not built** — [`m10-attribution.md`](m10-attribution.md). Scheduled ahead of M8 and M9 at the user's request, and the ordering is the point: M8 needs a fal.ai account and M9 needs GVC to exist, while this needs neither and gets more expensive every day it waits. Attribution captured at ingest costs a form field; attribution reconstructed later means opening every file by hand, and for material gathered from the open web the answer is often gone. Three decisions were taken with the user and are recorded with their rejected alternatives in that document: structured fields over a free-text credit, embedded metadata written directly while AI may only suggest, and clips inheriting from the parent with per-field override. | +| M10 Attribution | **Complete** — eight columns, the embedded-metadata harvester, read-time inheritance through a clip's parent, the keyword index, the library filters, the Source tab, and grounded AI suggestions. Specified and recorded in [`m10-attribution.md`](m10-attribution.md), including the two bugs real files caught that a stubbed tag dictionary would not have (EXIF `DateTimeOriginal` lives in the Exif sub-IFD, and PDF dates are prefixed and separator-less), and why the `asset_fts` rebuild needed its own Alembic revision with a repopulate. Scheduled ahead of M8 and M9 at the user's request: both need an external dependency this did not, and attribution captured at ingest costs a form field where attribution reconstructed later means opening every file by hand. | | **M8–M9** | **Not started.** Both need an external dependency M7 did not (fal.ai for M8, GVC itself for M9). | Non-milestone PRs, so a `git log` that does not match the table above still makes sense: diff --git a/frontend/src/api/assets.ts b/frontend/src/api/assets.ts index b1d4088..3a840b7 100644 --- a/frontend/src/api/assets.ts +++ b/frontend/src/api/assets.ts @@ -1,5 +1,6 @@ import client from '@/api/client' import type { Tag } from '@/api/tags' +import type { ActivityJob } from '@/api/transcripts' /** * The asset library API. @@ -49,6 +50,31 @@ export interface Asset { /** Batch-loaded for a page by the server, so reading this is free. */ tags: Tag[] + /** + * M10. Whose work this is — as opposed to `source`, which is how the file got here. + * + * These are the *resolved* values: a clip with nothing of its own carries what it + * inherited from its parent, so a clip of an attributed video shows a real credit + * rather than eight blanks. `attribution_inherited` names which of them came that + * way, so the panel can mark them instead of letting an inherited value look like + * something typed on the clip. + */ + source_url: string | null + creator: string | null + publisher: string | null + source_title: string | null + /** ISO 8601 partial: `YYYY`, `YYYY-MM` or `YYYY-MM-DD`. Partial on purpose — a book + * is from 1994 and nothing should invent a January 1st for it. */ + published_date: string | null + retrieved_at: string | null + license: string | null + /** An override. Null means the displayed `credit` is composed from the fields above. */ + credit_line: string | null + + /** Read-only: the line to display, composed unless `credit_line` overrides it. */ + credit: string + attribution_inherited: string[] + upload_date: string modified_date: string metadata_modified_date: string @@ -82,6 +108,15 @@ export interface ListAssetsParams { max_duration?: number uploaded_after?: string uploaded_before?: string + /** M10. Each of these resolves through a clip's parent server-side, so filtering by + * publisher returns the clips that inherit it as well as the asset itself. */ + creator?: string + publisher?: string + source_title?: string + published_after?: string + published_before?: string + /** True for "still missing a source" — how a backlog gets worked through. */ + unattributed?: boolean limit?: number offset?: number } @@ -97,8 +132,33 @@ export interface AssetUpdate { name?: string description?: string | null summary?: string | null + + /** M10. The only path that can *correct* attribution: the harvester fills blanks + * only, and an AI may propose but never write. */ + source_url?: string | null + creator?: string | null + publisher?: string | null + source_title?: string | null + published_date?: string | null + retrieved_at?: string | null + license?: string | null + credit_line?: string | null } +/** The attribution fields, in the order the panel shows them. */ +export const ATTRIBUTION_FIELDS = [ + 'creator', + 'source_title', + 'publisher', + 'published_date', + 'source_url', + 'license', + 'retrieved_at', + 'credit_line', +] as const + +export type AttributionField = (typeof ATTRIBUTION_FIELDS)[number] + interface DataResponse { data: T } @@ -144,4 +204,17 @@ export const assetsApi = { remove(id: string): Promise { return client.delete(`/assets/${id}`).then(() => undefined) }, + + /** + * Re-read embedded metadata for every file already in the library. + * + * For everything uploaded before M10 existed: the EXIF and ID3 have been sitting on + * disk the whole time and nothing ever looked. Returns the queued job, which shows up + * in the activity feed like any other. + */ + harvestAttribution(): Promise { + return client + .post>('/assets/harvest-attribution') + .then((r) => r.data.data) + }, } diff --git a/frontend/src/api/enrichment.ts b/frontend/src/api/enrichment.ts index 2872f18..83fd339 100644 --- a/frontend/src/api/enrichment.ts +++ b/frontend/src/api/enrichment.ts @@ -22,16 +22,26 @@ interface ListResponse { export interface Suggestion { id: string asset_id: string - kind: 'tag' | 'title' + kind: 'tag' | 'title' | 'attribution' value: string status: 'pending' | 'accepted' | 'rejected' + + /** M10, and null on every other kind. An attribution row carries JSON in `value`; + * the server decodes it so the same shape is not parsed in two places. */ + field: string | null + proposed_value: string | null + /** What the model quoted as its reason. Attribution may only ever be *suggested*, + * and a reviewer who cannot see what it read is not reviewing anything — so this + * is shown, not hidden behind a tooltip. */ + evidence: string | null } /** 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. */ -export type BulkAction = 'describe' | 'summarize' | 'autotag' | 'embed' | 'extract_text' +export type BulkAction = + 'describe' | 'summarize' | 'autotag' | 'embed' | 'extract_text' | 'attribute' /** One readable chunk of a document, in the unit that document naturally has. */ export interface DocumentPage { @@ -68,6 +78,14 @@ export const enrichmentApi = { .then((r) => r.data.data) }, + /** Propose where this came from. Writes nothing — every proposal arrives as a + * suggestion carrying the evidence it was read from. */ + attribute(assetId: string): Promise { + return client + .post>(`/assets/${assetId}/attribute`) + .then((r) => r.data.data) + }, + extractText(assetId: string): Promise { return client .post>(`/assets/${assetId}/extract-text`) diff --git a/frontend/src/components/AssetDetail.tsx b/frontend/src/components/AssetDetail.tsx index cd054da..2c6e50e 100644 --- a/frontend/src/components/AssetDetail.tsx +++ b/frontend/src/components/AssetDetail.tsx @@ -6,6 +6,7 @@ import { Link2, Mic, Pencil, + Quote, ScanText, Scissors, Tags as TagsIcon, @@ -25,6 +26,7 @@ import { useTagStore } from '@/stores/tags' import { formatBytes, formatDate, formatDimensions, formatDuration } from '@/utils/format' import { useAutoGrow } from '@/utils/useAutoGrow' import AssetThumb from '@/components/AssetThumb' +import AttributionPanel from '@/components/AttributionPanel' import ClipEditor from '@/components/ClipEditor' import DocumentTextPanel from '@/components/DocumentTextPanel' import EmbedButton from '@/components/EmbedButton' @@ -495,6 +497,9 @@ export default function AssetDetail({ + {/* The resolved credit, so it is readable without opening the Source tab — and + visible on a clip, which shows what it inherited. */} + {usage && usage.total_events > 0 && ( , + }, { id: 'info', label: 'Info', icon: Info, content: info }, ] diff --git a/frontend/src/components/AssetThumb.test.tsx b/frontend/src/components/AssetThumb.test.tsx index f2dbb2b..890a774 100644 --- a/frontend/src/components/AssetThumb.test.tsx +++ b/frontend/src/components/AssetThumb.test.tsx @@ -2,6 +2,7 @@ import { render, screen } from '@testing-library/react' import { describe, expect, it } from 'vitest' import AssetThumb from '@/components/AssetThumb' import type { Asset } from '@/api/assets' +import { noAttribution } from '@/test-fixtures' function makeAsset(overrides: Partial = {}): Asset { return { @@ -9,6 +10,7 @@ function makeAsset(overrides: Partial = {}): Asset { name: 'Test', description: null, summary: null, + ...noAttribution, asset_type: 'image', source: 'local_upload', parent_asset_id: null, diff --git a/frontend/src/components/AttributionPanel.test.tsx b/frontend/src/components/AttributionPanel.test.tsx new file mode 100644 index 0000000..8fae602 --- /dev/null +++ b/frontend/src/components/AttributionPanel.test.tsx @@ -0,0 +1,186 @@ +import { render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import AttributionPanel from '@/components/AttributionPanel' +import { useLibraryStore } from '@/stores/library' +import type { Asset } from '@/api/assets' +import { noAttribution } from '@/test-fixtures' + +function asset(overrides: Partial = {}): Asset { + return { + id: 'a1', + name: 'Interview', + description: null, + summary: null, + ...noAttribution, + asset_type: 'video', + source: 'local_upload', + parent_asset_id: null, + in_point: null, + out_point: null, + original_name: 'interview.mp4', + mime_type: 'video/mp4', + file_format: 'mp4', + size_bytes: 100, + duration_seconds: 60, + width: 1920, + height: 1080, + codec: 'h264', + file_url: null, + thumb_url: null, + missing: false, + tags: [], + upload_date: '2026-01-01T00:00:00', + modified_date: '2026-01-01T00:00:00', + metadata_modified_date: '2026-01-01T00:00:00', + ...overrides, + } +} + +const update = vi.fn().mockResolvedValue(undefined) + +beforeEach(() => { + update.mockClear() + useLibraryStore.setState({ update }) +}) + +describe('AttributionPanel', () => { + it('shows the fields that are filled in', () => { + render() + + expect(screen.getByLabelText('Creator')).toHaveValue('Jane Doe') + expect(screen.getByLabelText('Publisher')).toHaveValue('BBC') + }) + + it('previews the composed credit as you type', async () => { + const user = userEvent.setup() + render() + + await user.type(screen.getByLabelText('Creator'), 'Jane Doe') + await user.type(screen.getByLabelText('Publisher'), 'BBC') + + expect(screen.getByText('Jane Doe — BBC')).toBeInTheDocument() + }) + + it('prefers a typed credit line over the composition', async () => { + const user = userEvent.setup() + render() + + await user.type(screen.getByLabelText('Credit line'), 'Courtesy of the BBC') + + expect(screen.getByText('Courtesy of the BBC')).toBeInTheDocument() + expect(screen.queryByText('BBC', { selector: 'p' })).not.toBeInTheDocument() + }) + + it('sends only the fields that changed', async () => { + const user = userEvent.setup() + render() + + await user.type(screen.getByLabelText('Publisher'), 'BBC') + await user.click(screen.getByRole('button', { name: 'Save' })) + + // Not `creator` as well: the API stamps every key it receives as human-written, so + // resending an untouched field would mark it hand-verified when it was not. + await waitFor(() => expect(update).toHaveBeenCalledWith('a1', { publisher: 'BBC' })) + }) + + it('clears a field by sending null rather than an empty string', async () => { + const user = userEvent.setup() + render() + + await user.clear(screen.getByLabelText('Publisher')) + await user.click(screen.getByRole('button', { name: 'Save' })) + + await waitFor(() => expect(update).toHaveBeenCalledWith('a1', { publisher: null })) + }) + + it('cannot be saved until something changes', () => { + render() + expect(screen.getByRole('button', { name: 'Save' })).toBeDisabled() + }) + + it('refuses a malformed published date before it reaches the server', async () => { + const user = userEvent.setup() + render() + + await user.type(screen.getByLabelText('Published'), 'summer 1994') + + expect(screen.getByText('Use YYYY, YYYY-MM or YYYY-MM-DD.')).toBeInTheDocument() + expect(screen.getByRole('button', { name: 'Save' })).toBeDisabled() + expect(update).not.toHaveBeenCalled() + }) + + it('accepts a bare year', async () => { + const user = userEvent.setup() + render() + + await user.type(screen.getByLabelText('Published'), '1994') + + expect(screen.queryByText('Use YYYY, YYYY-MM or YYYY-MM-DD.')).not.toBeInTheDocument() + expect(screen.getByRole('button', { name: 'Save' })).toBeEnabled() + }) + + it('marks an inherited value as inherited', () => { + render( + + ) + + expect(screen.getByText('inherited from the original')).toBeInTheDocument() + }) + + it('does not mark a value the asset carries itself', () => { + render() + expect(screen.queryByText(/inherited from/)).not.toBeInTheDocument() + }) + + it('copies the credit', async () => { + // Spied rather than replaced: userEvent.setup() installs its own clipboard stub and + // defines it non-configurably, so assigning over it silently does nothing and the + // spy never sees the call. + const user = userEvent.setup() + const writeText = vi.spyOn(navigator.clipboard, 'writeText').mockResolvedValue() + + render( + + ) + + await user.click(screen.getByRole('button', { name: 'Copy this credit' })) + + await waitFor(() => expect(writeText).toHaveBeenCalledWith('Jane Doe — BBC')) + }) + + it('keeps the draft when a save is refused', async () => { + update.mockRejectedValueOnce(new Error('nope')) + const user = userEvent.setup() + render() + + await user.type(screen.getByLabelText('Publisher'), 'BBC') + await user.click(screen.getByRole('button', { name: 'Save' })) + + expect(await screen.findByText(/Could not save/)).toBeInTheDocument() + expect(screen.getByLabelText('Publisher')).toHaveValue('BBC') + }) + + it('does not let Escape reach the panel behind it', async () => { + const onKeyDown = vi.fn() + const user = userEvent.setup() + render( +
+ +
+ ) + + await user.click(screen.getByLabelText('Creator')) + await user.keyboard('{Escape}') + + // DetailDock listens for Escape to close the whole asset panel; abandoning a + // half-typed credit must not also shut the asset. + expect(onKeyDown).not.toHaveBeenCalled() + }) +}) diff --git a/frontend/src/components/AttributionPanel.tsx b/frontend/src/components/AttributionPanel.tsx new file mode 100644 index 0000000..389df10 --- /dev/null +++ b/frontend/src/components/AttributionPanel.tsx @@ -0,0 +1,324 @@ +import { useEffect, useMemo, useState } from 'react' +import type { KeyboardEvent } from 'react' +import { Check, Copy, Link2, ScanSearch } from 'lucide-react' +import type { Asset, AssetUpdate, AttributionField } from '@/api/assets' +import { enrichmentApi } from '@/api/enrichment' +import EnrichmentButton from '@/components/EnrichmentButton' +import SuggestionPanel from '@/components/SuggestionPanel' +import { useLibraryStore } from '@/stores/library' +import { useSavedFlash } from '@/utils/useSavedFlash' + +/** + * Where an asset's content came from, so it can be credited when it is used. + * + * `Asset.source` says how the file arrived — uploaded, generated, cut from something + * else. This is the other question: whose work it is. See docs/m10-attribution.md. + * + * The values arriving on `asset` are already *resolved*: a clip with nothing of its own + * carries what it inherited from its parent, and `attribution_inherited` names which. + * Those render as inherited rather than as ordinary values, because an inherited value + * that looked typed is one a user would "correct" here — silently detaching that field + * from the source, so that fixing the parent later no longer reaches it. + */ + +interface FieldSpec { + name: AttributionField + label: string + placeholder: string + hint?: string + type?: 'text' | 'url' | 'date' +} + +const FIELDS: FieldSpec[] = [ + { + name: 'creator', + label: 'Creator', + placeholder: 'Author, photographer, speaker, director', + }, + { + name: 'source_title', + label: 'Source title', + placeholder: 'The programme, film, article or book this is part of', + }, + { + name: 'publisher', + label: 'Publisher', + placeholder: 'Outlet, channel, studio, imprint', + }, + { + name: 'published_date', + label: 'Published', + placeholder: 'YYYY, YYYY-MM or YYYY-MM-DD', + hint: 'A year alone is fine — better than inventing a day nobody knows.', + }, + { + name: 'source_url', + label: 'Source URL', + placeholder: 'https://…', + type: 'url', + }, + { + name: 'license', + label: 'Licence / rights', + placeholder: 'CC BY 4.0, © the BBC, unknown', + }, +] + +/** Accepted by the server: a whole year, a year and month, or a full date. */ +const ISO_PARTIAL_DATE = /^\d{4}(?:-(?:0[1-9]|1[0-2])(?:-(?:0[1-9]|[12]\d|3[01]))?)?$/ + +type Draft = Record + +function draftFrom(asset: Asset): Draft { + return { + creator: asset.creator ?? '', + source_title: asset.source_title ?? '', + publisher: asset.publisher ?? '', + published_date: asset.published_date ?? '', + source_url: asset.source_url ?? '', + license: asset.license ?? '', + retrieved_at: asset.retrieved_at ?? '', + credit_line: asset.credit_line ?? '', + } +} + +/** + * The same composition the server performs, so the preview updates as you type. + * + * Duplicated from `app/attribution.py::compose_credit` on purpose: the alternative is a + * round trip per keystroke to show a line that is pure formatting. `credit` from the + * server remains authoritative — this only ever renders an unsaved draft. + */ +function composeCredit(draft: Draft): string { + return [ + draft.creator, + draft.source_title, + draft.publisher, + draft.published_date, + draft.license, + ] + .map((value) => value.trim()) + .filter(Boolean) + .join(' — ') +} + +interface Props { + asset: Asset +} + +export default function AttributionPanel({ asset }: Props) { + const update = useLibraryStore((s) => s.update) + const [draft, setDraft] = useState(() => draftFrom(asset)) + const [saving, setSaving] = useState(false) + const [error, setError] = useState(null) + const [saved, flashSaved] = useSavedFlash() + const [copied, setCopied] = useState(false) + + // Re-seed when the panel is pointed at a different asset, or when the server's copy + // changes underneath it — a harvest job filling blanks is exactly that case. + useEffect(() => { + setDraft(draftFrom(asset)) + setError(null) + // Deliberately not `[asset]`: the store hands back a new object on every update, so + // depending on its identity would re-seed the draft mid-edit and discard whatever + // was half-typed. Re-seeding on a different asset, or on a server-side change to + // this one (a harvest filling blanks), is the whole intent. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [asset.id, asset.metadata_modified_date]) + + const inherited = useMemo( + () => new Set(asset.attribution_inherited), + [asset.attribution_inherited] + ) + + const dirty = useMemo(() => { + const current = draftFrom(asset) + return (Object.keys(current) as AttributionField[]).some( + (key) => draft[key] !== current[key] + ) + }, [asset, draft]) + + const dateValid = + !draft.published_date.trim() || ISO_PARTIAL_DATE.test(draft.published_date.trim()) + + // The override when there is one, else the live composition — the same precedence the + // server applies, so what is previewed is what will be stored. + const preview = draft.credit_line.trim() || composeCredit(draft) + + const set = (name: AttributionField, value: string) => + setDraft((previous) => ({ ...previous, [name]: value })) + + const save = async () => { + if (!dirty || !dateValid) return + setSaving(true) + setError(null) + try { + // Only what changed. The API stamps every key it receives as human-written, so + // sending all eight would mark the seven you did not touch as hand-verified — + // including the ones a harvest filled, which is the distinction `provenance` + // exists to keep. + const current = draftFrom(asset) + const changes: AssetUpdate = {} + for (const key of Object.keys(current) as AttributionField[]) { + if (draft[key] !== current[key]) changes[key] = draft[key].trim() || null + } + + await update(asset.id, changes) + flashSaved() + } catch { + // The store restores the server's version and surfaces the message; leaving the + // draft alone means a rejected edit is still on screen to fix. + setError('Could not save. Your changes are still here.') + } finally { + setSaving(false) + } + } + + const copyCredit = async () => { + const line = asset.credit || preview + if (!line) return + try { + await navigator.clipboard.writeText(line) + setCopied(true) + setTimeout(() => setCopied(false), 1500) + } catch { + // Clipboard access needs a secure context and a user gesture. Prompting with the + // text beats a button that silently does nothing. + window.prompt('Copy this credit', line) + } + } + + // Escape must not reach DetailDock's window listener, which closes the whole panel — + // abandoning a half-typed credit should not also shut the asset. + const swallowEscape = (event: KeyboardEvent) => { + if (event.key === 'Escape') event.stopPropagation() + } + + return ( +
+

+ Where this came from, so it can be credited when you use it. +

+ + {/* Proposes, never writes — the one enrichment that may not, because a wrong + citation credits somebody else's work to the wrong outlet in a field that then + looks finished. Absent on a clip: it has no bytes and no transcript of its own + to read, and it inherits its original's attribution anyway. */} + {!asset.parent_asset_id && ( + + )} + + + + {FIELDS.map((field) => ( +
+
+ + {inherited.has(field.name) && ( + + inherited from {asset.parent_asset_id ? 'the original' : 'its source'} + + )} +
+ set(field.name, e.target.value)} + /> + {field.name === 'published_date' && !dateValid && ( +

+ Use YYYY, YYYY-MM or YYYY-MM-DD. +

+ )} + {field.hint && dateValid && field.name === 'published_date' && ( +

{field.hint}

+ )} +
+ ))} + +
+ + set('credit_line', e.target.value)} + /> +

+ Leave this empty and it is built from the fields above, so correcting one keeps + the credit right. +

+
+ + {preview && ( +
+
+
+

Credit

+

+ {preview} +

+
+ +
+
+ )} + + {asset.source_url && ( + + + Open the source + + )} + + {error &&

{error}

} + +
+ + {saved && ( + + + Saved + + )} +
+
+ ) +} diff --git a/frontend/src/components/FilterBar.tsx b/frontend/src/components/FilterBar.tsx index a2fda48..8630375 100644 --- a/frontend/src/components/FilterBar.tsx +++ b/frontend/src/components/FilterBar.tsx @@ -63,6 +63,12 @@ export default function FilterBar() { const toggleTag = useLibraryStore((s) => s.toggleTag) const setCategoryFilter = useLibraryStore((s) => s.setCategoryFilter) const setSourceFilter = useLibraryStore((s) => s.setSourceFilter) + const creatorFilter = useLibraryStore((s) => s.creatorFilter) + const publisherFilter = useLibraryStore((s) => s.publisherFilter) + const unattributedOnly = useLibraryStore((s) => s.unattributedOnly) + const setCreatorFilter = useLibraryStore((s) => s.setCreatorFilter) + const setPublisherFilter = useLibraryStore((s) => s.setPublisherFilter) + const setUnattributedOnly = useLibraryStore((s) => s.setUnattributedOnly) const setDurationRange = useLibraryStore((s) => s.setDurationRange) const setUploadedRange = useLibraryStore((s) => s.setUploadedRange) const clearFilters = useLibraryStore((s) => s.clearFilters) @@ -128,7 +134,10 @@ export default function FilterBar() { (categoryFilter ? 1 : 0) + (sourceFilter ? 1 : 0) + (minDuration !== null || maxDuration !== null ? 1 : 0) + - (uploadedAfter || uploadedBefore ? 1 : 0) + (uploadedAfter || uploadedBefore ? 1 : 0) + + (creatorFilter ? 1 : 0) + + (publisherFilter ? 1 : 0) + + (unattributedOnly ? 1 : 0) const anyActive = activeCount > 0 || Boolean(query.trim()) || Boolean(typeFilter) const durationLabel = () => { @@ -236,6 +245,50 @@ export default function FilterBar() { + {/* M10. Both resolve through a clip's parent server-side, so filtering by + publisher returns the clips cut from an attributed video as well as the + video itself. Committed on blur or Enter rather than per keystroke: each + change reloads the library, and a request per character would be one + request per character. */} +
+ + setCreatorFilter(value || null)} + /> +
+ +
+ + setPublisherFilter(value || null)} + /> +
+ +
+ Attribution + +

+ A clip showing its original's credit is not missing one. +

+
+
Duration (seconds)
@@ -320,6 +373,24 @@ export default function FilterBar() { onClear={() => setUploadedRange(null, null)} /> )} + {creatorFilter && ( + setCreatorFilter(null)} + /> + )} + {publisherFilter && ( + setPublisherFilter(null)} + /> + )} + {unattributedOnly && ( + setUnattributedOnly(false)} + /> + )}
))} + {attributions.map((suggestion) => ( +
+

+ {FIELD_LABELS[suggestion.field ?? ''] ?? suggestion.field} +

+
+ + {suggestion.proposed_value} + + +
+ {/* Shown rather than tucked away: attribution is the one enrichment that may + never write directly, and the evidence is what makes accepting a judgement + rather than a reflex. */} + {suggestion.evidence && ( +

+ “{suggestion.evidence}” +

+ )} +
+ ))} + {tags.length > 0 && (

Tags

@@ -176,12 +220,16 @@ function Decide({ disabled: boolean onDecide: (s: Suggestion, accepted: boolean) => Promise }) { + // An attribution row's `value` is the JSON payload the server encoded, so naming the + // button after it would read out a blob to a screen reader — and to anyone hovering. + const label = suggestion.proposed_value ?? suggestion.value + return (