diff --git a/README.md b/README.md index 9eda69c..c470888 100644 --- a/README.md +++ b/README.md @@ -41,9 +41,10 @@ user row. The changes that makes possible are specified in in audio and video come back with a timestamp you can jump straight to. - **Clips and sub-videos** — mark a range non-destructively (no new file, always in step with its parent), or physically extract it as a standalone asset. -- **Opt-in AI enrichment** — transcription, image description, summaries and tag - suggestions. Suggestions are reviewed, never applied silently, and a field you edited - by hand is not overwritten by a later AI run. +- **Opt-in AI enrichment** — transcription, text extraction from PDFs and Office + documents, image description, summaries and tag suggestions. Suggestions are reviewed, + never applied silently, and a field you edited by hand is not overwritten by a later AI + run. - **AI asset creation** — generate images from images, and video from one or more images, with the prompt and base assets recorded so a result stays reproducible. - **Cost visibility** — an estimate before anything paid runs, and the real cost tracked diff --git a/backend/alembic/versions/20260916_1111_add_documentpage_table.py b/backend/alembic/versions/20260916_1111_add_documentpage_table.py new file mode 100644 index 0000000..ee3d714 --- /dev/null +++ b/backend/alembic/versions/20260916_1111_add_documentpage_table.py @@ -0,0 +1,55 @@ +"""add documentpage table + +Readable text pulled out of a document, so `summarize` and `autotag` have something to +read for a PDF or a Word file. Until now they refused: `enrichment/source.py` raised +`NoSourceMaterial` saying so out loud, because the `extract_text` job the plan lists had +never been built. + +One row per page, slide, sheet or chunk rather than a column on `asset`, because +SQLModel selects every column of every row and the library listing selects assets by the +page — a 200-page PDF's text in an asset column would be read from disk to render a +thumbnail grid that never looks at it. No foreign key to `asset`, matching +`transcriptsegment`; the delete cascade is explicit in `services/assets.py`. + +Revision ID: 56ac14e89a0c +Revises: c4954d1715e7 +Create Date: 2026-09-16 11:11:08.578568+00:00 +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +import sqlmodel + + +revision: str = '56ac14e89a0c' +down_revision: Union[str, None] = 'c4954d1715e7' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.create_table('documentpage', + sa.Column('id', sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column('asset_id', sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column('user_id', sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column('idx', sa.Integer(), nullable=False), + sa.Column('page_number', sa.Integer(), nullable=True), + sa.Column('label', sqlmodel.sql.sqltypes.AutoString(), nullable=True), + sa.Column('text', sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.PrimaryKeyConstraint('id') + ) + with op.batch_alter_table('documentpage', schema=None) as batch_op: + batch_op.create_index(batch_op.f('ix_documentpage_asset_id'), ['asset_id'], unique=False) + batch_op.create_index('ix_documentpage_asset_idx', ['asset_id', 'idx'], unique=False) + batch_op.create_index(batch_op.f('ix_documentpage_user_id'), ['user_id'], unique=False) + + + +def downgrade() -> None: + with op.batch_alter_table('documentpage', schema=None) as batch_op: + batch_op.drop_index(batch_op.f('ix_documentpage_user_id')) + batch_op.drop_index('ix_documentpage_asset_idx') + batch_op.drop_index(batch_op.f('ix_documentpage_asset_id')) + + op.drop_table('documentpage') diff --git a/backend/app/enrichment/autotag.py b/backend/app/enrichment/autotag.py index 36c82b9..062a801 100644 --- a/backend/app/enrichment/autotag.py +++ b/backend/app/enrichment/autotag.py @@ -78,6 +78,11 @@ def _prompt(session: Session, asset: Asset, material: source.SourceMaterial) -> 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.") else: diff --git a/backend/app/enrichment/bulk.py b/backend/app/enrichment/bulk.py index b51ffca..dcf395e 100644 --- a/backend/app/enrichment/bulk.py +++ b/backend/app/enrichment/bulk.py @@ -19,9 +19,15 @@ from sqlmodel import Session -from app.enrichment import autotag, describe, embed, summarize +from app.enrichment import autotag, describe, embed, extract_text, summarize from app.models.asset import Asset -from app.models.job import KIND_AUTOTAG, KIND_DESCRIBE, KIND_EMBED, KIND_SUMMARIZE +from app.models.job import ( + KIND_AUTOTAG, + KIND_DESCRIBE, + KIND_EMBED, + KIND_EXTRACT_TEXT, + KIND_SUMMARIZE, +) from app.providers.base import ProviderUnavailable logger = logging.getLogger(__name__) @@ -40,6 +46,10 @@ KIND_SUMMARIZE: summarize.run, KIND_AUTOTAG: autotag.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. + # It is also the action most worth having in bulk — documents arrive by the folder. + KIND_EXTRACT_TEXT: extract_text.run, } # A selection has to be reviewable before it is run, and it is the thing standing between @@ -140,4 +150,5 @@ def inner(_stage, _pct, _detail="", *, stage=stage, pct=pct, detail=detail): KIND_SUMMARIZE: "Summarising", KIND_AUTOTAG: "Suggesting tags", KIND_EMBED: "Embedding", + KIND_EXTRACT_TEXT: "Reading text", } diff --git a/backend/app/enrichment/describe.py b/backend/app/enrichment/describe.py index 0cc2d29..8e4b166 100644 --- a/backend/app/enrichment/describe.py +++ b/backend/app/enrichment/describe.py @@ -71,6 +71,11 @@ def _prompt(asset: Asset, material: source.SourceMaterial) -> str: "visual — who is on screen, the setting — which the transcript cannot " "tell you." ) + elif material.kind == source.FROM_DOCUMENT: + header = "Text of the document" + if material.truncated: + header += " (the opening portion only — it continues beyond this)" + parts.append(f"{header}:\n\n{material.text}") elif material.kind == source.FROM_POSTER: parts.append( "There is no transcript, so a single still frame from the video is " diff --git a/backend/app/enrichment/extract_text.py b/backend/app/enrichment/extract_text.py new file mode 100644 index 0000000..c02bfff --- /dev/null +++ b/backend/app/enrichment/extract_text.py @@ -0,0 +1,361 @@ +"""The extract_text job: a document in, readable text out. + +The odd one out of the enrichment set. It calls no provider, spends nothing, and nobody +asks for it because they want to read its output — they want `summarize` and `autotag` +to stop refusing. Those jobs read whatever `enrichment/source.py` hands them, and for a +document it handed them a refusal, because this module did not exist. + +Runs on a worker thread, so it owns its session and reports progress through the +callback the queue hands it — which is also the cancellation checkpoint. +""" + +from __future__ import annotations + +import csv +import io +import logging +from pathlib import Path +from typing import Callable, Iterator, Optional + +from sqlmodel import Session, col, delete + +from app.ingest.filetypes import TYPE_DOCUMENT, extension_of +from app.models.asset import Asset +from app.models.document import DocumentPage +from app.storage import build_storage + +logger = logging.getLogger(__name__) + +Progress = Callable[..., None] + +# Formats with a unit of their own — a page, a slide, a sheet. +EXT_PDF = ".pdf" +EXT_DOCX = ".docx" +EXT_PPTX = ".pptx" +EXT_XLSX = ".xlsx" +PLAIN_EXTENSIONS = frozenset({".txt", ".md", ".csv"}) + +EXTRACTABLE_EXTENSIONS = frozenset( + {EXT_PDF, EXT_DOCX, EXT_PPTX, EXT_XLSX} | PLAIN_EXTENSIONS +) + +# How much text goes in one row, for the formats with no unit of their own. A row is the +# chunk that will be embedded individually when full-document search lands, and embedders +# cap somewhere near 8k tokens, so this leaves headroom rather than sitting on the limit. +# Splitting happens on a paragraph boundary where there is one within reach. +MAX_CHUNK_CHARS = 4000 + +# Past this, stop reading. A document that long is a data dump rather than something +# anyone wants summarised, and the LLM jobs truncate at 60k anyway — the difference this +# bound makes is to the database and to how long the job holds a worker thread. +MAX_TOTAL_CHARS = 2_000_000 + + +class TextExtractionError(Exception): + """Something the user can be told about.""" + + +def extractable(asset: Asset) -> bool: + if asset.asset_type != TYPE_DOCUMENT or not asset.storage_key: + return False + return extension_of(asset.original_name or "") in EXTRACTABLE_EXTENSIONS + + +def run(session: Session, asset: Asset, progress: Progress) -> str: + """Extract one document's text. Returns what to show in the activity feed.""" + if asset.asset_type != TYPE_DOCUMENT or not asset.storage_key: + raise TextExtractionError("Only documents can have their text extracted") + + extension = extension_of(asset.original_name or "") + if extension not in EXTRACTABLE_EXTENSIONS: + # Named rather than generic: "we cannot read .rtf" is actionable — convert it and + # upload again — where "extraction failed" invites a re-run that cannot work. + raise TextExtractionError( + f"Reading text out of {extension or 'this format'} files is not supported. " + "Convert it to PDF, DOCX or plain text and upload it again." + ) + + progress("Reading the file", 5, "") + + storage = build_storage() + with storage.materialise(asset.storage_key) as path: + try: + chunks = list(_extract(path, extension, progress)) + except TextExtractionError: + raise + except Exception as exc: # noqa: BLE001 - upstream parsers raise their own zoo + raise TextExtractionError( + "The file could not be read. It may be corrupt, password-protected or " + "not really the format its name claims." + ) from exc + + if not chunks: + raise TextExtractionError( + "No text could be read from this document. A scan or a photographed page " + "holds pictures of words rather than words, which needs OCR." + ) + + progress("Saving", 90, "") + stored = _store(session, asset, chunks) + + unit = _unit_name(extension, stored) + return f"{stored} {unit}" + + +class _Chunk: + __slots__ = ("page_number", "label", "text") + + def __init__(self, text: str, *, page_number: Optional[int] = None, label: Optional[str] = None): + self.text = text + self.page_number = page_number + self.label = label + + +def _extract(path: Path, extension: str, progress: Progress) -> Iterator[_Chunk]: + if extension == EXT_PDF: + yield from _from_pdf(path, progress) + elif extension == EXT_DOCX: + yield from _from_docx(path, progress) + elif extension == EXT_PPTX: + yield from _from_pptx(path, progress) + elif extension == EXT_XLSX: + yield from _from_xlsx(path, progress) + else: + yield from _from_plain(path, progress) + + +def _from_pdf(path: Path, progress: Progress) -> Iterator[_Chunk]: + """One row per page, which is the one format where that is simply true. + + pypdfium2 rather than pypdf: it is already a dependency for first-page thumbnails, + and `ingest/thumbnails.py` records why it was chosen over the AGPL and + poppler-dependent alternatives. Adding a second PDF library to read what this one + already has open would reopen a settled argument. + """ + import pypdfium2 + + document = pypdfium2.PdfDocument(str(path)) + try: + total = len(document) + budget = MAX_TOTAL_CHARS + for index in range(total): + progress("Extracting text", _percent(index, total), f"page {index + 1} of {total}") + page = document[index] + textpage = page.get_textpage() + try: + # `get_text_bounded()`, not `get_text_range()`: the latter warns that a + # call with default arguments is redirected here anyway, and a warning + # per page of a long PDF is a lot of noise for the same bytes. + text = textpage.get_text_bounded() + finally: + textpage.close() + page.close() + text = _tidy(text) + if not text: + continue + yield _Chunk(text, page_number=index + 1, label=f"Page {index + 1}") + budget -= len(text) + if budget <= 0: + logger.warning("Stopped reading %s at page %s: too much text", path.name, index + 1) + return + finally: + document.close() + + +def _from_docx(path: Path, progress: Progress) -> Iterator[_Chunk]: + """Paragraphs, chunked — a .docx has no pages until something renders it. + + Table cells are read as well as paragraphs: a specification or an invoice can carry + most of its meaning in a table, and skipping them reads as "extraction produced + nothing" for a file that is visibly full of words. + """ + import docx + + progress("Extracting text", 20, "") + document = docx.Document(str(path)) + + blocks = [p.text for p in document.paragraphs] + for table in document.tables: + for row in table.rows: + cells = [cell.text.strip() for cell in row.cells] + if any(cells): + blocks.append("\t".join(cells)) + + yield from _chunked(blocks, progress) + + +def _from_pptx(path: Path, progress: Progress) -> Iterator[_Chunk]: + """One row per slide, including the speaker notes. + + The notes are where the argument usually lives — a slide says "Q3 Revenue" and the + notes say why it fell — so leaving them out would summarise the headings alone. + """ + from pptx import Presentation + + presentation = Presentation(str(path)) + slides = list(presentation.slides) + total = len(slides) + + for index, slide in enumerate(slides): + progress("Extracting text", _percent(index, total), f"slide {index + 1} of {total}") + parts: list[str] = [] + for shape in slide.shapes: + if shape.has_text_frame: + parts.append(shape.text_frame.text) + if slide.has_notes_slide: + notes = slide.notes_slide.notes_text_frame + if notes is not None and notes.text.strip(): + parts.append(f"Notes: {notes.text}") + text = _tidy("\n".join(parts)) + if not text: + continue + yield _Chunk(text, page_number=index + 1, label=f"Slide {index + 1}") + + +def _from_xlsx(path: Path, progress: Progress) -> Iterator[_Chunk]: + """One row per worksheet, cells joined into lines. + + `read_only=True` streams rather than building the whole workbook in memory, which + matters for the spreadsheets people actually keep. Formulas are not evaluated — + `data_only=True` would return the last cached value, or None for a sheet never opened + in Excel, so the formula text is the more honest thing to read. + """ + import openpyxl + + workbook = openpyxl.load_workbook(str(path), read_only=True, data_only=False) + try: + sheets = workbook.worksheets + total = len(sheets) + for index, sheet in enumerate(sheets): + progress("Extracting text", _percent(index, total), f"sheet {index + 1} of {total}") + lines: list[str] = [] + for row in sheet.iter_rows(values_only=True): + cells = [str(value) for value in row if value is not None] + if cells: + lines.append("\t".join(cells)) + text = _tidy("\n".join(lines)) + if not text: + continue + yield _Chunk( + text, + page_number=index + 1, + label=f"Sheet {sheet.title!r}" if sheet.title else f"Sheet {index + 1}", + ) + finally: + workbook.close() + + +def _from_plain(path: Path, progress: Progress) -> Iterator[_Chunk]: + """Decode and chunk. + + UTF-8 with `errors="replace"`, and no encoding detection: there is no chardet in this + tree, and guessing wrong silently is worse than a few replacement characters in a + summary. A CSV is read through the csv module so quoted cells containing newlines do + not split a record in half. + """ + progress("Extracting text", 20, "") + raw = path.read_bytes() + if len(raw) > MAX_TOTAL_CHARS: + raw = raw[:MAX_TOTAL_CHARS] + body = raw.decode("utf-8", errors="replace") + + if extension_of(path.name) == ".csv": + reader = csv.reader(io.StringIO(body)) + blocks = ["\t".join(row) for row in reader if any(cell.strip() for cell in row)] + else: + blocks = body.split("\n") + + yield from _chunked(blocks, progress) + + +def _chunked(blocks: list[str], progress: Progress) -> Iterator[_Chunk]: + """Group blocks into rows of at most MAX_CHUNK_CHARS, never splitting a block. + + `page_number` stays null here, deliberately: these formats have no page a reader + could turn to, and numbering the chunks would put a number in a column whose whole + contract is that it means the page the reader sees. + """ + current: list[str] = [] + size = 0 + part = 0 + total_chars = 0 + + def flush() -> Optional[_Chunk]: + nonlocal current, size, part + text = _tidy("\n".join(current)) + current = [] + size = 0 + if not text: + return None + part += 1 + return _Chunk(text, label=f"Part {part}") + + for block in blocks: + block = block.rstrip() + if size and size + len(block) > MAX_CHUNK_CHARS: + chunk = flush() + if chunk is not None: + yield chunk + total_chars += len(chunk.text) + if total_chars >= MAX_TOTAL_CHARS: + return + progress("Extracting text", min(80, 20 + part), f"part {part}") + current.append(block) + size += len(block) + 1 + + chunk = flush() + if chunk is not None: + yield chunk + + +def _store(session: Session, asset: Asset, chunks: list[_Chunk]) -> int: + """Replace what was there. + + Wholesale, unlike a transcript re-run, which preserves hand-edited segments. Nothing + edits extracted text by hand — there is no editor for it — so there is nothing to + protect, and keeping old rows around would double the document on every re-run. + """ + session.exec(delete(DocumentPage).where(col(DocumentPage.asset_id) == asset.id)) + + for index, chunk in enumerate(chunks): + session.add( + DocumentPage( + asset_id=asset.id, + user_id=asset.user_id, + idx=index, + page_number=chunk.page_number, + label=chunk.label, + text=chunk.text, + ) + ) + + session.commit() + return len(chunks) + + +def _tidy(text: str) -> str: + """Collapse the whitespace a PDF extractor leaves behind, keeping paragraph breaks.""" + lines = [line.strip() for line in (text or "").replace("\r\n", "\n").replace("\r", "\n").split("\n")] + out: list[str] = [] + for line in lines: + if line: + out.append(line) + elif out and out[-1]: + out.append("") + return "\n".join(out).strip() + + +def _percent(index: int, total: int) -> int: + if total <= 0: + return 50 + return 10 + int(70 * index / total) + + +def _unit_name(extension: str, count: int) -> str: + if extension == EXT_PDF: + return "page" if count == 1 else "pages" + if extension == EXT_PPTX: + return "slide" if count == 1 else "slides" + if extension == EXT_XLSX: + return "sheet" if count == 1 else "sheets" + return "section" if count == 1 else "sections" diff --git a/backend/app/enrichment/source.py b/backend/app/enrichment/source.py index bb4921b..d02c12c 100644 --- a/backend/app/enrichment/source.py +++ b/backend/app/enrichment/source.py @@ -21,6 +21,7 @@ from app.ingest.filetypes import TYPE_DOCUMENT, TYPE_IMAGE from app.models.asset import Asset +from app.models.document import DocumentPage from app.models.transcript import TranscriptSegment from app.providers.base import Image from app.storage.base import StorageError @@ -31,6 +32,7 @@ FROM_TRANSCRIPT = "transcript" FROM_IMAGE = "image" FROM_POSTER = "poster" +FROM_DOCUMENT = "document" # A transcript of a long interview is far more text than any model needs to summarise it, # and the input side is what a long job costs. Cut at a generous ceiling rather than @@ -72,6 +74,21 @@ def transcript_text(session: Session, asset: Asset) -> str: return "\n".join(s.text.strip() for s in segments if (s.text or "").strip()).strip() +def document_text(session: Session, asset: Asset) -> str: + """The asset's extracted text as one block, in order. + + Ordered by `idx` for the same reason `transcript_text` is: nothing in SQL promises + to read rows back the way they went in, and a summary built from shuffled pages is + wrong in a way that is hard to notice. + """ + pages = session.exec( + select(DocumentPage) + .where(DocumentPage.asset_id == asset.id) + .order_by(col(DocumentPage.idx)) + ).all() + return "\n\n".join(p.text.strip() for p in pages if (p.text or "").strip()).strip() + + def _poster_image(asset: Asset) -> Optional[Image]: """The video poster ingest already generated, if it is still there. @@ -146,6 +163,22 @@ def gather( if image: return SourceMaterial(kind=FROM_IMAGE, images=[image]) + # A document with extracted text. Deliberately **above** the poster branch, not down + # with the old refusal: every PDF gets a first-page thumbnail at ingest, so a PDF + # sent to a vision provider used to match the poster branch and be summarised from a + # picture of its cover. That is the same mistake the transcript rule at the top of + # this module exists to prevent, one format over — the words beat the one rendered + # page, and the fallback only applies when there are no words. + if asset.asset_type == TYPE_DOCUMENT: + body = document_text(session, asset) + if body: + truncated = len(body) > MAX_TRANSCRIPT_CHARS + return SourceMaterial( + kind=FROM_DOCUMENT, + text=body[:MAX_TRANSCRIPT_CHARS] if truncated else body, + truncated=truncated, + ) + # A video with no transcript: the poster frame is all there is. Deliberately after # the transcript branch, never instead of it. if supports_images: @@ -154,13 +187,12 @@ def gather( return SourceMaterial(kind=FROM_POSTER, images=[poster]) if asset.asset_type == TYPE_DOCUMENT: - # `extract_text` is specified in plan-of-attack and has no KIND_ constant and no - # module, so a document carries no readable text yet. Named explicitly so this - # reads as a known gap rather than as an asset that mysteriously cannot be - # enriched. + # Reached when extraction has not run, or ran and found nothing — a scan holds + # pictures of words rather than words. Says which, because the two have different + # answers: run it, versus this file needs OCR. raise NoSourceMaterial( - "Reading text out of documents is not built yet, so there is nothing to " - "read for this file." + "No text has been read out of this document yet. Run Extract text on it " + "first — and if that finds nothing, the file is a scan and needs OCR." ) raise NoSourceMaterial( diff --git a/backend/app/enrichment/summarize.py b/backend/app/enrichment/summarize.py index 4bbe8b7..50846fd 100644 --- a/backend/app/enrichment/summarize.py +++ b/backend/app/enrichment/summarize.py @@ -56,6 +56,11 @@ def _prompt(asset: Asset, material: source.SourceMaterial) -> str: # material it was never shown. header += " (the opening portion only — it continues beyond this)" parts.append(f"{header}:\n\n{material.text}") + elif material.kind == source.FROM_DOCUMENT: + header = "Text of the document" + if material.truncated: + header += " (the opening portion only — it continues beyond this)" + parts.append(f"{header}:\n\n{material.text}") elif material.kind == source.FROM_POSTER: parts.append( "No transcript is available, so a single still frame from the video is " diff --git a/backend/app/jobs/enrichment.py b/backend/app/jobs/enrichment.py index 4e9cd44..22f54e5 100644 --- a/backend/app/jobs/enrichment.py +++ b/backend/app/jobs/enrichment.py @@ -22,6 +22,8 @@ from app.enrichment.autotag import run as run_autotag from app.enrichment.bulk import run as run_bulk from app.enrichment.describe import run as run_describe +from app.enrichment.extract_text import TextExtractionError +from app.enrichment.extract_text import run as run_extract_text from app.enrichment.source import NoSourceMaterial from app.enrichment.summarize import run as run_summarize from app.enrichment.transcribe import TranscriptionError @@ -42,6 +44,7 @@ KIND_BULK_ENRICH, KIND_DESCRIBE, KIND_EMBED, + KIND_EXTRACT_TEXT, KIND_SUMMARIZE, KIND_TRANSCRIBE, LIBRARY_KINDS, @@ -219,6 +222,8 @@ def _run_job(job_id: str) -> None: elif job.kind == KIND_EMBED: count = run_embed(session, asset, progress) detail = f"{count} vector{'' if count == 1 else 's'}" + elif job.kind == KIND_EXTRACT_TEXT: + detail = run_extract_text(session, asset, progress) elif job.kind == KIND_SUMMARIZE: detail = run_summarize(session, asset, progress) elif job.kind == KIND_AUTOTAG: @@ -255,6 +260,11 @@ def _run_job(job_id: str) -> None: ) if job.kind == KIND_TRANSCRIBE: + # Not KIND_EXTRACT_TEXT: `embed` vectorises an asset's name, description + # and summary plus its transcript segments, and document pages are in + # none of those. Chaining it here would queue a job that does no new + # work and put a row in the activity feed saying so. It belongs here the + # day page bodies are embedded — see the search gap in plan-of-attack. _chain_embedding(session, asset) except JobCancelled: @@ -286,6 +296,12 @@ def _run_job(job_id: str) -> None: set_fields(session, job, status="error", stage="", error_message=message) logger.info("Enrichment job %s failed: %s", job_id, message) + except TextExtractionError as exc: + message = str(exc) + _mark_asset_failed(session, asset, job) + set_fields(session, job, status="error", stage="", error_message=message) + logger.info("Enrichment job %s failed: %s", job_id, message) + except TranscriptionError as exc: message = str(exc) _mark_asset_failed(session, asset, job) diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index c1ac4df..8d7e3ec 100644 --- a/backend/app/models/__init__.py +++ b/backend/app/models/__init__.py @@ -10,6 +10,7 @@ """ from app.models.asset import Asset +from app.models.document import DocumentPage from app.models.embedding import Embedding from app.models.job import EnrichmentJob from app.models.provider import AIProvider @@ -24,6 +25,7 @@ "AIProvider", "Asset", "AssetTag", + "DocumentPage", "Embedding", "EnrichmentJob", "Suggestion", diff --git a/backend/app/models/document.py b/backend/app/models/document.py new file mode 100644 index 0000000..a0d6120 --- /dev/null +++ b/backend/app/models/document.py @@ -0,0 +1,45 @@ +import uuid +from typing import Optional + +from sqlalchemy import Index +from sqlmodel import Field, SQLModel + + +class DocumentPage(SQLModel, table=True): + """One readable chunk of a document, in the unit that document naturally has. + + Rows rather than a column on `Asset`, for a reason that only shows up at scale: + SQLModel loads every column of a row it selects, and the library listing selects + assets by the page. A 200-page PDF's text in an `Asset` column would be dragged + through every one of those listings to render a thumbnail grid that never looks at + it. + + Shaped after `TranscriptSegment` minus the timings, and for the same reason it gives: + a chunk is the unit that will be individually indexed and individually embedded when + full-document search lands. Storing one blob per asset would foreclose that. + """ + + # Reading extracted text is always "every chunk of this asset, in order", which is + # the same access pattern transcripts have and the same index shape. + __table_args__ = (Index("ix_documentpage_asset_idx", "asset_id", "idx"),) + + id: str = Field(default_factory=lambda: str(uuid.uuid4()), primary_key=True) + asset_id: str = Field(index=True) + user_id: str = Field(index=True) + + # Position in the document. Always present, and the only thing ordering depends on — + # `page_number` cannot serve, because half the formats here do not have one. + idx: int = Field(default=0) + + # The real page number, where the format has real pages: PDF pages and PPTX slides + # do, and are 1-based to match what the reader sees. A DOCX has no pagination at all + # until something renders it, and a plain text file has none ever, so this is null + # for them rather than a chunk index wearing a page number's name. + page_number: Optional[int] = None + + # What to call this chunk in the UI — "Page 3", "Slide 7", "Sheet 'Q3 Revenue'", + # "Part 2". Stored rather than derived because a spreadsheet's unit is named by the + # author and cannot be reconstructed from an index. + label: Optional[str] = None + + text: str = Field(default="") diff --git a/backend/app/models/job.py b/backend/app/models/job.py index b9e81ed..128696d 100644 --- a/backend/app/models/job.py +++ b/backend/app/models/job.py @@ -27,6 +27,10 @@ KIND_AUTOTAG = "autotag" KIND_EMBED = "embed" KIND_BACKFILL_EMBEDDINGS = "backfill_embeddings" +# Pulling readable text out of a document so the LLM jobs have something to read. It is +# the odd one out of the per-asset set: it calls no provider, costs nothing, and its +# output is an input to the other three rather than something a user reads directly. +KIND_EXTRACT_TEXT = "extract_text" # One action applied across a chosen set of assets. Which action, and which assets, # live in `EnrichmentJob.payload` — see there for why it is one job and not N. KIND_BULK_ENRICH = "bulk_enrich" @@ -39,6 +43,7 @@ KIND_SUMMARIZE, KIND_AUTOTAG, KIND_EMBED, + KIND_EXTRACT_TEXT, } ) diff --git a/backend/app/routers/enrichment.py b/backend/app/routers/enrichment.py index c942896..180a93b 100644 --- a/backend/app/routers/enrichment.py +++ b/backend/app/routers/enrichment.py @@ -8,15 +8,18 @@ from __future__ import annotations +from typing import Optional + from fastapi import APIRouter, Depends, HTTPException, status from pydantic import BaseModel, ConfigDict -from sqlmodel import Session +from sqlmodel import Session, col, select from app.auth import CurrentUser from app.database import get_session from app.embeddings import build_embedder from app.enrichment import bulk from app.enrichment.describe import describable +from app.enrichment.extract_text import extractable from app.enrichment.summarize import summarisable from app.jobs import enrichment as enrichment_jobs from app.jobs.registry import KINDS @@ -26,8 +29,10 @@ KIND_BULK_ENRICH, KIND_DESCRIBE, KIND_EMBED, + KIND_EXTRACT_TEXT, KIND_SUMMARIZE, ) +from app.models.document import DocumentPage from app.models.suggestion import Suggestion from app.providers import build_provider from app.schemas import DataResponse, ListResponse @@ -177,6 +182,84 @@ def start_describe( return DataResponse(data=KINDS["enrichment"].to_activity(job)) +class DocumentPageRead(BaseModel): + model_config = ConfigDict(from_attributes=True) + + id: str + idx: int + page_number: Optional[int] = None + label: Optional[str] = None + text: str + + +@router.post( + "/{asset_id}/extract-text", response_model=DataResponse[ActivityJobRead], status_code=202 +) +def start_extract_text( + asset_id: str, + user: CurrentUser, + session: Session = Depends(get_session), +) -> DataResponse[ActivityJobRead]: + """Queue a text extraction over one document. + + No `_require_provider`: this is the one enrichment action that calls nothing and + costs nothing. Requiring a provider here would make configuring an LLM a + precondition for reading a PDF, which it is not — and the whole point of this job is + to be the step that runs *before* one is any use. + """ + asset = _owned_asset(asset_id, user.id, session) + + if not extractable(asset): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail={ + "code": "not_extractable", + "message": ( + "Text can only be read out of PDF, Word, PowerPoint, Excel and " + "plain-text files." + ), + }, + ) + + if enrichment_jobs.active_job(session, asset.id, KIND_EXTRACT_TEXT) is not None: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail={ + "code": "already_running", + "message": "This document's text is already being read", + }, + ) + + job = enrichment_jobs.submit(session, asset, KIND_EXTRACT_TEXT) + return DataResponse(data=KINDS["enrichment"].to_activity(job)) + + +@router.get("/{asset_id}/text", response_model=ListResponse[DocumentPageRead]) +def list_document_text( + asset_id: str, + user: CurrentUser, + session: Session = Depends(get_session), +) -> ListResponse[DocumentPageRead]: + """What extraction read out of this document, in order. + + Exists so extracted text is visible rather than merely stored. Text the user cannot + see is indistinguishable from text that was never extracted, and this repository has + enough of that already. + """ + _owned_asset(asset_id, user.id, session) + rows = session.exec( + select(DocumentPage) + .where(DocumentPage.asset_id == asset_id) + .order_by(col(DocumentPage.idx)) + ).all() + return ListResponse( + data=[DocumentPageRead.model_validate(r) for r in rows], + total=len(rows), + limit=len(rows), + offset=0, + ) + + @router.get("/{asset_id}/suggestions", response_model=ListResponse[SuggestionRead]) def list_suggestions( asset_id: str, diff --git a/backend/app/services/assets.py b/backend/app/services/assets.py index 2d0dbde..8625afe 100644 --- a/backend/app/services/assets.py +++ b/backend/app/services/assets.py @@ -29,6 +29,7 @@ from app.ingest.probe import probe from app.models.asset import Asset from app.models.suggestion import Suggestion +from app.models.document import DocumentPage from app.models.transcript import TranscriptSegment from app.schemas_assets import AssetRead, AssetTagRead from app.search import fts, vectors @@ -194,6 +195,7 @@ def delete_asset(session: Session, storage: LocalStorage, asset: Asset) -> None: tags.detach_all_from_asset(session, asset_id) session.exec(delete(Suggestion).where(col(Suggestion.asset_id) == asset_id)) session.exec(delete(TranscriptSegment).where(col(TranscriptSegment.asset_id) == asset_id)) + session.exec(delete(DocumentPage).where(col(DocumentPage.asset_id) == asset_id)) session.delete(asset) session.commit() diff --git a/backend/requirements.txt b/backend/requirements.txt index 2aff595..3901f15 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -24,3 +24,13 @@ Pillow==10.4.0 # AGPL and would put obligations on anyone deploying this, and pdf2image needs a # poppler binary on the host. pypdfium2==4.30.0 +# Text extraction for `extract_text`, so enrichment can read documents and not just +# media. PDFs and plain text need nothing new — pypdfium2 above already opens a PDF, and +# reusing it keeps that licensing argument settled rather than reopening it for pypdf. +# These three cover the Office formats, are pure-Python wheels, and pull one compiled +# dependency between them (lxml, via docx and pptx), which ships cp313 wheels — the +# thing to check before adding anything here, since a source build on the pinned 3.13 is +# how Pillow and numpy break. +python-docx==1.2.0 +python-pptx==1.0.2 +openpyxl==3.1.5 diff --git a/backend/tests/fixtures/sample_document.docx b/backend/tests/fixtures/sample_document.docx new file mode 100644 index 0000000..101ac24 Binary files /dev/null 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 new file mode 100644 index 0000000..c9228b7 Binary files /dev/null and b/backend/tests/fixtures/sample_document.pptx differ diff --git a/backend/tests/fixtures/sample_document.txt b/backend/tests/fixtures/sample_document.txt new file mode 100644 index 0000000..a6715e9 --- /dev/null +++ b/backend/tests/fixtures/sample_document.txt @@ -0,0 +1,3 @@ +Gecko Asset Manager + +A library for video, images and documents. diff --git a/backend/tests/fixtures/sample_document.xlsx b/backend/tests/fixtures/sample_document.xlsx new file mode 100644 index 0000000..2384477 Binary files /dev/null and b/backend/tests/fixtures/sample_document.xlsx differ diff --git a/backend/tests/make_fixtures.py b/backend/tests/make_fixtures.py index c41887e..2f82a5b 100644 --- a/backend/tests/make_fixtures.py +++ b/backend/tests/make_fixtures.py @@ -72,10 +72,64 @@ def main() -> None: import pypdfium2 # noqa: F401 - imported to fail early if it is missing _write_minimal_pdf(FIXTURES / "sample_document.pdf") + _write_office_documents() print("wrote:", ", ".join(sorted(p.name for p in FIXTURES.iterdir()))) +def _write_office_documents() -> None: + """The Office and plain-text fixtures `extract_text` reads. + + All four carry the same "Gecko Asset Manager" phrase the PDF does, so one assertion + shape covers every format, plus something format-specific — a table, a speaker note, + a second sheet — so a reader that silently skips half a file is caught. + """ + import docx + import openpyxl + from pptx import Presentation + from pptx.util import Inches + + document = docx.Document() + document.add_paragraph("Gecko Asset Manager") + document.add_paragraph("A library for video, images and documents.") + # A table, because a .docx that carries its content in one is the case a + # paragraphs-only reader gets wrong while looking like it worked. + table = document.add_table(rows=2, cols=2) + table.cell(0, 0).text = "Format" + table.cell(0, 1).text = "Reader" + table.cell(1, 0).text = "docx" + table.cell(1, 1).text = "python-docx" + document.save(FIXTURES / "sample_document.docx") + + presentation = Presentation() + slide = presentation.slides.add_slide(presentation.slide_layouts[5]) + slide.shapes.title.text = "Gecko Asset Manager" + box = slide.shapes.add_textbox(Inches(1), Inches(2), Inches(4), Inches(1)) + box.text_frame.text = "Slide one body text." + # Speaker notes, which is where a deck's actual argument usually lives. + slide.notes_slide.notes_text_frame.text = "Remember to mention the search demo." + second = presentation.slides.add_slide(presentation.slide_layouts[5]) + second.shapes.title.text = "Second slide" + presentation.save(FIXTURES / "sample_document.pptx") + + workbook = openpyxl.Workbook() + sheet = workbook.active + sheet.title = "Overview" + sheet["A1"] = "Gecko Asset Manager" + sheet["A2"] = "Assets" + sheet["B2"] = 42 + # A second sheet, so "one row per worksheet" has something to be wrong about. + other = workbook.create_sheet("Costs") + other["A1"] = "Enrichment spend" + other["B1"] = 12.5 + workbook.save(FIXTURES / "sample_document.xlsx") + + (FIXTURES / "sample_document.txt").write_text( + "Gecko Asset Manager\n\nA library for video, images and documents.\n", + encoding="utf-8", + ) + + def _write_minimal_pdf(target: Path) -> None: """A valid one-page PDF, written by hand. diff --git a/backend/tests/test_extract_text.py b/backend/tests/test_extract_text.py new file mode 100644 index 0000000..f15415a --- /dev/null +++ b/backend/tests/test_extract_text.py @@ -0,0 +1,324 @@ +"""The extract_text job — the step that stops documents refusing enrichment. + +Two behaviours carry most of the cases: what each format's unit is (a PDF page, a slide, +a worksheet, a chunk), and the precedence question this job created — a PDF has a +first-page thumbnail, so something has to decide between reading its words and looking +at a picture of its cover. +""" + +from pathlib import Path + +import httpx +import pytest +from sqlmodel import col, select + +from app.enrichment import extract_text, source, summarize +from app.jobs import enrichment as enrichment_jobs +from app.jobs.runner import JobCancelled +from app.models.asset import Asset +from app.models.document import DocumentPage +from app.models.job import EnrichmentJob, KIND_EXTRACT_TEXT +from app.providers import _upstream +from tests.test_summarize import configure_provider, sent_prompt + +FIXTURES = Path(__file__).parent / "fixtures" +TEST_USER = "user-under-test" + +MIME = { + ".pdf": "application/pdf", + ".docx": "application/vnd.openxmlformats-officedocument.wordprocessingml.document", + ".pptx": "application/vnd.openxmlformats-officedocument.presentationml.presentation", + ".xlsx": "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + ".txt": "text/plain", +} + + +def upload_document(client, extension=".pdf", name=None): + filename = name or f"sample_document{extension}" + return client.post( + "/api/assets", + files=[ + ( + "files", + ( + filename, + (FIXTURES / f"sample_document{extension}").read_bytes(), + MIME[extension], + ), + ) + ], + ).json()["created"][0] + + +@pytest.fixture(name="upstream") +def upstream_fixture(monkeypatch): + """Answer every completion with a canned summary, and record what was asked. + + A local copy rather than a shared one, matching test_describe.py — the assertion + helpers are imported, the fixture is not. + """ + state = {"calls": []} + + 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": "A quarterly report."}], + "usage": {"input_tokens": 120, "output_tokens": 20}, + }, + ) + + monkeypatch.setattr(_upstream, "post_json", fake_post_json) + return state + + +def pages_of(session, asset_id): + return session.exec( + select(DocumentPage) + .where(DocumentPage.asset_id == asset_id) + .order_by(col(DocumentPage.idx)) + ).all() + + +@pytest.mark.parametrize( + "extension,expected_chunks", + [(".pdf", 1), (".docx", 1), (".pptx", 2), (".xlsx", 2), (".txt", 1)], +) +def test_every_supported_format_yields_its_text(library, session, extension, expected_chunks): + created = upload_document(library, extension) + asset = session.get(Asset, created["id"]) + + extract_text.run(session, asset, lambda *a, **k: None) + + pages = pages_of(session, asset.id) + assert len(pages) == expected_chunks + assert "Gecko Asset Manager" in pages[0].text + + +def test_a_pdf_page_carries_its_real_page_number(library, session): + created = upload_document(library, ".pdf") + asset = session.get(Asset, created["id"]) + + extract_text.run(session, asset, lambda *a, **k: None) + + page = pages_of(session, asset.id)[0] + assert page.page_number == 1 + assert page.label == "Page 1" + + +def test_a_docx_has_no_page_number_because_it_has_no_pages(library, session): + """Numbering the chunks would put a number in a column whose contract is that it + means the page the reader can turn to.""" + created = upload_document(library, ".docx") + asset = session.get(Asset, created["id"]) + + extract_text.run(session, asset, lambda *a, **k: None) + + page = pages_of(session, asset.id)[0] + assert page.page_number is None + assert page.label == "Part 1" + + +def test_a_docx_table_is_read_as_well_as_its_paragraphs(library, session): + """A file that carries its content in a table is the case a paragraphs-only reader + gets wrong while looking like it worked.""" + created = upload_document(library, ".docx") + asset = session.get(Asset, created["id"]) + + extract_text.run(session, asset, lambda *a, **k: None) + + assert "python-docx" in pages_of(session, asset.id)[0].text + + +def test_speaker_notes_are_read(library, session): + """A deck's argument usually lives in the notes; the slide says only 'Q3 Revenue'.""" + created = upload_document(library, ".pptx") + asset = session.get(Asset, created["id"]) + + extract_text.run(session, asset, lambda *a, **k: None) + + assert "search demo" in pages_of(session, asset.id)[0].text + + +def test_each_worksheet_is_its_own_row_named_after_itself(library, session): + created = upload_document(library, ".xlsx") + asset = session.get(Asset, created["id"]) + + extract_text.run(session, asset, lambda *a, **k: None) + + pages = pages_of(session, asset.id) + assert [p.label for p in pages] == ["Sheet 'Overview'", "Sheet 'Costs'"] + assert "Enrichment spend" in pages[1].text + + +def test_an_unsupported_format_is_refused_by_name(library, session): + """"We cannot read .rtf" is actionable; "extraction failed" invites a re-run that + cannot work.""" + created = upload_document(library, ".txt", name="notes.rtf") + asset = session.get(Asset, created["id"]) + + with pytest.raises(extract_text.TextExtractionError, match=r"\.rtf"): + extract_text.run(session, asset, lambda *a, **k: None) + + +def test_a_re_run_replaces_rather_than_appends(library, session): + created = upload_document(library, ".pptx") + asset = session.get(Asset, created["id"]) + + extract_text.run(session, asset, lambda *a, **k: None) + extract_text.run(session, asset, lambda *a, **k: None) + + assert len(pages_of(session, asset.id)) == 2 + + +def test_cancelling_mid_extraction_writes_nothing(library, session): + """The progress callback is the cancellation checkpoint, so a cancel lands between + pages — and must not leave half a document behind.""" + created = upload_document(library, ".pptx") + asset = session.get(Asset, created["id"]) + + def cancel_immediately(*args, **kwargs): + raise JobCancelled("job-under-test") + + with pytest.raises(JobCancelled): + extract_text.run(session, asset, cancel_immediately) + + assert pages_of(session, asset.id) == [] + + +def test_extracted_text_beats_the_first_page_thumbnail(library, session, upstream): + """The precedence trap this job created. + + Every PDF gets a first-page thumbnail at ingest, so before the document branch was + placed above the poster branch a PDF sent to a vision provider was summarised from a + picture of its cover instead of its words. + """ + created = upload_document(library, ".pdf") + asset = session.get(Asset, created["id"]) + extract_text.run(session, asset, lambda *a, **k: None) + asset.thumb_key = "thumbs/pretend-this-exists.jpg" + session.commit() + configure_provider(session, supports_images=True) + + material = source.gather(session, asset, supports_images=True) + + assert material.kind == source.FROM_DOCUMENT + assert "Gecko Asset Manager" in material.text + + summarize.run(session, asset, lambda *a, **k: None) + assert "Text of the document" in sent_prompt(upstream) + + +def test_deleting_the_asset_takes_its_pages_with_it(library, session): + created = upload_document(library, ".pdf") + asset = session.get(Asset, created["id"]) + extract_text.run(session, asset, lambda *a, **k: None) + + assert library.delete(f"/api/assets/{asset.id}").status_code == 204 + + assert pages_of(session, created["id"]) == [] + + +# --- the API ---------------------------------------------------------------------- + + +def test_the_endpoint_queues_a_job_without_a_provider(library, session): + """The one enrichment action that must not require an LLM: it is the step that runs + before one is any use.""" + created = upload_document(library, ".pdf") + + response = library.post(f"/api/assets/{created['id']}/extract-text") + + assert response.status_code == 202 + assert response.json()["data"]["action"] == KIND_EXTRACT_TEXT + + +def test_a_file_nothing_can_read_is_refused_before_it_is_queued(library): + created = upload_document(library, ".txt", name="notes.rtf") + + response = library.post(f"/api/assets/{created['id']}/extract-text") + + assert response.status_code == 400 + assert response.json()["detail"]["code"] == "not_extractable" + + +def test_a_video_cannot_have_its_text_extracted(library, session): + created = upload_document(library, ".pdf") + asset = session.get(Asset, created["id"]) + asset.asset_type = "video" + session.commit() + + response = library.post(f"/api/assets/{asset.id}/extract-text") + + assert response.status_code == 400 + + +def test_two_runs_at_once_are_refused(library, session): + created = upload_document(library, ".pdf") + + first = library.post(f"/api/assets/{created['id']}/extract-text") + second = library.post(f"/api/assets/{created['id']}/extract-text") + + assert first.status_code == 202 + assert second.status_code == 409 + assert second.json()["detail"]["code"] == "already_running" + + +def test_somebody_elses_asset_is_not_there(library): + response = library.post("/api/assets/does-not-exist/extract-text") + assert response.status_code == 404 + + +def test_the_text_endpoint_returns_the_pages_in_order(library, session): + created = upload_document(library, ".xlsx") + asset = session.get(Asset, created["id"]) + extract_text.run(session, asset, lambda *a, **k: None) + + body = library.get(f"/api/assets/{asset.id}/text").json() + + assert body["total"] == 2 + assert [row["label"] for row in body["data"]] == ["Sheet 'Overview'", "Sheet 'Costs'"] + + +def test_the_text_endpoint_is_empty_before_extraction_runs(library, session): + created = upload_document(library, ".pdf") + + body = library.get(f"/api/assets/{created['id']}/text").json() + + assert body["data"] == [] + + +def test_the_worker_runs_it_end_to_end(library, session, monkeypatch): + created = upload_document(library, ".pptx") + job = library.post(f"/api/assets/{created['id']}/extract-text").json()["data"] + + queue = enrichment_jobs.queue() + monkeypatch.setattr(queue, "engine", session.get_bind()) + enrichment_jobs._run_job(job["id"]) + session.expire_all() + + row = session.get(EnrichmentJob, job["id"]) + assert row.status == "done" + assert row.detail == "2 slides" + assert len(pages_of(session, created["id"])) == 2 + + +def test_a_worker_failure_is_reported_in_the_users_words(library, session, monkeypatch): + """Not "crashed": TextExtractionError has to be in the dispatcher's exception ladder + or the ladder's bare Exception branch swallows the sentence.""" + created = upload_document(library, ".pdf") + job = library.post(f"/api/assets/{created['id']}/extract-text").json()["data"] + + def explode(*args, **kwargs): + raise extract_text.TextExtractionError("The file could not be read.") + + monkeypatch.setattr(enrichment_jobs, "run_extract_text", explode) + queue = enrichment_jobs.queue() + monkeypatch.setattr(queue, "engine", session.get_bind()) + enrichment_jobs._run_job(job["id"]) + session.expire_all() + + row = session.get(EnrichmentJob, job["id"]) + assert row.status == "error" + assert row.error_message == "The file could not be read." diff --git a/backend/tests/test_summarize.py b/backend/tests/test_summarize.py index e277041..6f9a806 100644 --- a/backend/tests/test_summarize.py +++ b/backend/tests/test_summarize.py @@ -16,6 +16,7 @@ from app.enrichment import source, summarize from app.jobs import enrichment as enrichment_jobs from app.models.asset import Asset +from app.models.document import DocumentPage from app.models.job import EnrichmentJob, KIND_SUMMARIZE from app.models.provider import AIProvider from app.models.transcript import TranscriptSegment @@ -71,6 +72,21 @@ def add_transcript(session, asset_id, lines): session.commit() +def add_document_text(session, asset_id, chunks): + for idx, text in enumerate(chunks): + session.add( + DocumentPage( + asset_id=asset_id, + user_id=TEST_USER, + idx=idx, + page_number=idx + 1, + label=f"Page {idx + 1}", + text=text, + ) + ) + session.commit() + + @pytest.fixture(name="upstream") def upstream_fixture(monkeypatch): """Answer every completion with a canned summary, and record what was asked.""" @@ -149,9 +165,10 @@ def test_an_image_asset_with_a_text_only_provider_has_nothing_to_read(library, s summarize.run(session, asset, lambda *a, **k: None) -def test_a_document_says_the_gap_out_loud(library, session): - """`extract_text` is specified in plan-of-attack and does not exist, so this reads - as a known gap rather than a file that mysteriously cannot be enriched.""" +def test_a_document_with_no_extracted_text_says_which_step_is_missing(library, session): + """This used to assert that `extract_text` did not exist. It does now, so the + refusal has to distinguish the two reasons a document has nothing to read: nobody + has run extraction, or extraction ran and the file is a scan.""" created = _upload_image(library) asset = session.get(Asset, created["id"]) asset.asset_type = "document" @@ -159,10 +176,29 @@ def test_a_document_says_the_gap_out_loud(library, session): session.commit() configure_provider(session, supports_images=False) - with pytest.raises(source.NoSourceMaterial, match="documents is not built yet"): + with pytest.raises(source.NoSourceMaterial, match="Run Extract text on it first"): summarize.run(session, asset, lambda *a, **k: None) +def test_a_document_is_summarised_from_its_extracted_text(library, session, upstream): + """The point of the whole `extract_text` job: documents stop refusing.""" + created = _upload_image(library) + asset = session.get(Asset, created["id"]) + asset.asset_type = "document" + session.commit() + add_document_text(session, asset.id, ["The quarterly report on neuroweapons research."]) + configure_provider(session, supports_images=False) + + summarize.run(session, asset, lambda *a, **k: None) + + prompt = sent_prompt(upstream) + assert "The quarterly report on neuroweapons research." in prompt + # Not "Transcript of the recording", which is what all three jobs said about + # everything that was not an image before this branch existed. + assert "Text of the document" in prompt + assert "Transcript" not in prompt + + def test_a_very_long_transcript_is_cut_and_says_so(library, session, upstream): """Otherwise the model writes 'the talk concludes by…' about material it never saw.""" created = _upload_image(library) diff --git a/docs/m6-ai-enrichment.md b/docs/m6-ai-enrichment.md index 39afc52..2bfc0db 100644 --- a/docs/m6-ai-enrichment.md +++ b/docs/m6-ai-enrichment.md @@ -4,9 +4,10 @@ because they are what proves the chain works on real content. Written after reading `davior/gecko-notes` at `95ed2ca`. -Two gaps are recorded below and are *not* part of M6: `extract_text` does not exist, so -enrichment over documents is still blocked, and attribution (where an asset came from — -a URL, a film, a broadcast) has no model yet. +Two gaps were recorded below and were *not* part of M6. `extract_text` has since been +built, so enrichment over documents works; what is left of it is that the extracted text +is not itself searchable. Attribution — where an asset came from, a URL, a film, a +broadcast — still has no model. This exists because most of what M6 needs already works in gecko-notes, and a session that starts from `plan-of-attack.md` alone would design it from scratch instead. Read @@ -157,7 +158,7 @@ themselves: | Image | the image bytes | Needs `supports_images`; refused before sending otherwise | | Video or audio **with** a transcript | the transcript text | The case this section exists for. `describe` also gets the poster frame — see step 5 | | Video **without** a transcript | the poster frame | `ingest/thumbnails.py::_from_video` already produces one | -| Document | extracted text | **Nothing produces this yet** — see below | +| Document | extracted text | `extract_text` produces it. Above the poster row in precedence: a PDF has a cover thumbnail, and its words beat a picture of them | | Anything else | nothing | The job refuses rather than inventing from a filename | Two things this table makes visible that were previously implicit: @@ -166,11 +167,13 @@ Two things this table makes visible that were previously implicit: it has one. A single frame of a two-hour interview describes a person sitting down. The transcript describes what was said, which is what anyone searching is actually looking for. The frame is the fallback for silent or untranscribed video, not the primary. -- **`extract_text` does not exist.** `plan-of-attack.md` lists it as a job - (`pypdf`/`python-docx`/plain read) but there is no `KIND_EXTRACT_TEXT` in - `models/job.py` and no module for it. So a PDF currently has no text for `summarize` or - `autotag` to read, and enrichment over documents is blocked on building it. That is a - gap, not a decision. +- **~~`extract_text` does not exist.~~** It does now — `enrichment/extract_text.py`, + writing `DocumentPage` rows. PDF text comes from pypdfium2 rather than the pypdf the + plan named, because pypdfium2 is already a dependency for first-page thumbnails and + `ingest/thumbnails.py` records why it was chosen over the AGPL and poppler-dependent + alternatives; a second PDF library would reopen a settled argument. What is still open + is that those rows are indexed nowhere, so a document is findable by the summary + enrichment writes and not by its own body — see `plan-of-attack.md`'s outstanding list. ### One pass, not three diff --git a/docs/plan-of-attack.md b/docs/plan-of-attack.md index dba2898..c7f74a5 100644 --- a/docs/plan-of-attack.md +++ b/docs/plan-of-attack.md @@ -209,7 +209,7 @@ cancellation, heartbeats and restart recovery come nearly free. | `describe` | Vision LLM (Anthropic/OpenAI via the `AIProvider` pattern). The SRS says fal.ai for descriptions; fal is a generation platform and its captioning models are weaker than a vision LLM at the "what is in this image, in retrieval-useful words" task. Recommend the LLM; fal stays for *creating* media. | | `summarize` | LLM over transcript / extracted document text | | `autotag` | LLM proposing tags, stored `status="suggested"` until accepted (FR 9.1.4 — never applied silently) | -| `extract_text` | pypdf / python-docx / plain read. **Not built** — see Outstanding below | +| `extract_text` | pypdfium2 (PDF), python-docx / python-pptx / openpyxl (Office), stdlib decode (txt/md/csv). One `DocumentPage` row per page, slide, sheet or chunk. Calls no provider and bills nothing, which is why it is in bulk where transcription is not. | | `embed` | Runs after any of the above that produce text | Cost visibility (FR 8.1.4) reuses `UsageEvent` + `pricing.py` + `compute_fal_cost` @@ -323,7 +323,7 @@ it was written in does not survive the session. | M2 SSO & first deploy | Merged — [#4](https://github.com/davior/gam/pull/4) | | M5 Search | Merged — [#5](https://github.com/davior/gam/pull/5). Shipped unreachable; made configurable in #11, and reachable for a pre-existing library in #13 — **acceptance still not run, see below** | | 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` and attribution. | +| 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–M9** | **Not started.** M7 is the only one with no external dependency. | Non-milestone PRs, so a `git log` that does not match the table above still makes sense: @@ -345,13 +345,24 @@ from a decision, which is the distinction PR bodies do not preserve. `ingest/filetypes.py`, declared with no writer — as are `SOURCE_AI` and `SOURCE_GVC`, which are legitimately waiting on M8 and M9. `FilterBar` offers an "AI generated" source filter, wired end to end and tested, over a value nothing can currently set. -- **`extract_text` was specified and never built.** There is no `KIND_EXTRACT_TEXT` in - `models/job.py` and no module for it, so a PDF or a Word document has no text for - `summarize` or `autotag` to read and enrichment over documents is blocked outright. - M6's source-material table (`m6-ai-enrichment.md`) names it as the source for documents - and records the same gap. The job table above lists it as though it exists; it does not. - It is a gap, not a decision — M6 shipped around it by refusing rather than by inventing - a description from a filename. +- **~~`extract_text` was specified and never built.~~** Built. PDF, `.docx`, `.pptx`, + `.xlsx`, `.txt`/`.md`/`.csv`, one `DocumentPage` row per natural unit, and a + `FROM_DOCUMENT` branch in `enrichment/source.py` placed **above** the poster branch — + every PDF has a first-page thumbnail, so a document branch any lower would leave a PDF + summarised from a picture of its cover. + + Two parts of it remain, and neither is a decision: + - **Document text is not searchable.** The rows exist and are in neither `segment_fts` + nor the vector index, so a phrase on page 74 cannot be found. A document *is* + findable by the summary enrichment writes, because `apply_ai_metadata` reindexes — + which is easy to mistake for the body being indexed. Closing this means indexing page + bodies, embedding them, and teaching `search/hybrid.py` to resolve a hit id to either + a transcript segment or a page. `jobs/enrichment.py` carries the one-line + `_chain_embedding` change that goes with it, commented. + - **Legacy and OpenDocument formats refuse by name.** `.doc`, `.xls`, `.ppt`, `.rtf`, + `.odt`, `.ods`, `.odp` are in `DOCUMENT_EXTENSIONS` and nothing reads them. The + refusal says which format and what to do about it, rather than failing vaguely. + - A scan is refused too, and always will be without OCR, which is its own project. - **Attribution is not modelled.** Raised by the user, recorded here so it is not mistaken for a decision: an asset needs to carry where its content *came from* — a website URL, the film or programme a clip is taken from, the news outlet and date of a diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 5c593f7..2a31489 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -2,10 +2,12 @@ import { useEffect } from 'react' import { Navigate, Route, Routes } from 'react-router-dom' import AppShell from '@/components/AppShell' import RequireAuth from '@/components/RequireAuth' +import AssetView from '@/views/AssetView' import LibraryView from '@/views/LibraryView' import SearchView from '@/views/SearchView' import SettingsView from '@/views/SettingsView' import { loadConfig } from '@/api/config' +import { setUnauthorizedHandler } from '@/api/client' import { useAuthStore } from '@/stores/auth' export default function App() { @@ -23,6 +25,22 @@ export default function App() { }) }, [bootstrap]) + // Wired here rather than inside `api/client.ts`, which cannot import the auth store: + // the store imports `api/auth.ts`, which imports the client. A callback set from the + // one place that already depends on both keeps that cycle from existing. + useEffect(() => { + setUnauthorizedHandler(() => { + // Only for a session that *was* good. `bootstrap()` gets a 401 for every visitor + // who is not signed in, and that is the anonymous path RequireAuth already + // handles with a sign-in button — bouncing them to the Notes login instead would + // turn "you are not signed in" into a redirect nobody asked for. + const auth = useAuthStore.getState() + if (auth.status !== 'authenticated') return false + auth.signOut() + return true + }) + }, []) + return ( } /> @@ -56,6 +74,18 @@ export default function App() { } /> + {/* Above the catch-all, which otherwise swallows this into a silent redirect. + GN-4 specifies this path as the Notes→GAM asset reference. */} + + + + + + } + /> } /> ) diff --git a/frontend/src/api/client.test.ts b/frontend/src/api/client.test.ts index 12e0a3e..bbe048b 100644 --- a/frontend/src/api/client.test.ts +++ b/frontend/src/api/client.test.ts @@ -1,6 +1,10 @@ import { AxiosError } from 'axios' -import { describe, expect, it } from 'vitest' -import client, { apiErrorCode, apiErrorMessage } from '@/api/client' +import { afterEach, describe, expect, it } from 'vitest' +import client, { + apiErrorCode, + apiErrorMessage, + setUnauthorizedHandler, +} from '@/api/client' function axiosErrorWith(data: unknown, code?: string): AxiosError { const error = new AxiosError('Request failed', code) @@ -81,3 +85,103 @@ describe('query parameter serialisation', () => { ) }) }) + +describe('the unauthorized interceptor', () => { + /** Push a failure through the real response interceptor chain. */ + async function reject(error: AxiosError) { + const handlers = ( + client.interceptors.response as unknown as { + handlers: Array<{ rejected: (e: unknown) => Promise }> + } + ).handlers + for (const handler of handlers) { + if (handler?.rejected) { + await handler.rejected(error).catch(() => undefined) + } + } + } + + function status(code: number, detailCode?: string): AxiosError { + const error = new AxiosError('Request failed') + error.response = { + data: detailCode ? { detail: { code: detailCode, message: 'nope' } } : {}, + status: code, + statusText: '', + headers: {}, + config: {} as never, + } + return error + } + + afterEach(() => { + setUnauthorizedHandler(null) + }) + + it('calls the handler on a 401', async () => { + let called = 0 + setUnauthorizedHandler(() => { + called += 1 + return true + }) + + await reject(status(401, 'unauthorized')) + + expect(called).toBe(1) + }) + + it('fires once, so a polling store cannot trigger a redirect per tick', async () => { + let called = 0 + setUnauthorizedHandler(() => { + called += 1 + return true + }) + + await reject(status(401, 'unauthorized')) + await reject(status(401, 'unauthorized')) + await reject(status(401, 'unauthorized')) + + expect(called).toBe(1) + }) + + it('keeps its one shot when the handler declines', async () => { + // A 401 during bootstrap for a visitor who was never signed in is the anonymous + // path, not an expiry. Spending the shot on it would leave a real expiry later in + // the session unhandled. + let called = 0 + let acting = false + setUnauthorizedHandler(() => { + called += 1 + return acting + }) + + await reject(status(401, 'unauthorized')) + acting = true + await reject(status(401, 'unauthorized')) + + expect(called).toBe(2) + }) + + it('ignores a CSRF rejection, which says nothing about the session', async () => { + let called = 0 + setUnauthorizedHandler(() => { + called += 1 + return true + }) + + await reject(status(403, 'forbidden_origin')) + + expect(called).toBe(0) + }) + + it('ignores an ordinary 400', async () => { + let called = 0 + setUnauthorizedHandler(() => { + called += 1 + return true + }) + + await reject(status(400, 'not_extractable')) + + expect(called).toBe(0) + }) +}) diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 15b5f05..5b4893f 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -76,6 +76,54 @@ client.interceptors.request.use((config) => { return config }) +/** + * What to do when the session turns out to be over. + * + * A callback rather than an import, because importing the auth store here would close a + * cycle: the store imports `api/auth.ts`, which imports this module. `App.tsx` already + * depends on both and sets it on mount. + */ +type UnauthorizedHandler = () => boolean + +let onUnauthorized: UnauthorizedHandler | null = null +let handled = false + +export function setUnauthorizedHandler(handler: UnauthorizedHandler | null): void { + onUnauthorized = handler + handled = false +} + +/** + * Codes that mean "this session is over". + * + * The handler returns whether it acted, and only then is this treated as spent. A 401 + * during `bootstrap()` for a visitor who was never signed in is the ordinary anonymous + * case, not an expiry — burning the one shot on it would leave a real expiry later in + * the session with nothing to handle it. + */ +const SESSION_ENDED = new Set(['unauthorized', 'session_not_accepted']) + +client.interceptors.response.use( + (response) => response, + (error: unknown) => { + // Fires once. The activity store polls every two to ten seconds, so a session that + // expires mid-upload produces a stream of 401s — and `signOut` navigates away, so + // reacting to each one would fight the redirect it already started. + // + // Deliberately not 403 `forbidden_origin`: that is the CSRF guard refusing a + // cross-origin write, which says nothing about whether the session is still good. + // Signing someone out over it would turn a rejected request into a lost session. + if (!handled && onUnauthorized && axios.isAxiosError(error)) { + const status = error.response?.status + const code = apiErrorCode(error) + if (status === 401 && (code === null || SESSION_ENDED.has(code))) { + handled = onUnauthorized() + } + } + return Promise.reject(error) + } +) + /** * Pull a readable sentence out of a failure. * diff --git a/frontend/src/api/enrichment.ts b/frontend/src/api/enrichment.ts index 883ee81..bcd8f4d 100644 --- a/frontend/src/api/enrichment.ts +++ b/frontend/src/api/enrichment.ts @@ -29,8 +29,18 @@ export interface Suggestion { /** What a whole selection can be put through. Transcription is deliberately absent — * it is billed per minute of audio, and a mis-click over two hundred videos is an - * expensive way to discover it was on the menu. */ -export type BulkAction = 'describe' | 'summarize' | 'autotag' | 'embed' + * 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' + +/** One readable chunk of a document, in the unit that document naturally has. */ +export interface DocumentPage { + id: string + idx: number + page_number: number | null + label: string | null + text: string +} export const enrichmentApi = { summarize(assetId: string): Promise { @@ -51,6 +61,18 @@ export const enrichmentApi = { .then((r) => r.data.data) }, + extractText(assetId: string): Promise { + return client + .post>(`/assets/${assetId}/extract-text`) + .then((r) => r.data.data) + }, + + text(assetId: string): Promise { + return client + .get>(`/assets/${assetId}/text`) + .then((r) => r.data.data) + }, + suggestions(assetId: string): Promise { return client .get>(`/assets/${assetId}/suggestions`) diff --git a/frontend/src/components/ActivityIndicator.tsx b/frontend/src/components/ActivityIndicator.tsx index 77ef4a6..2810807 100644 --- a/frontend/src/components/ActivityIndicator.tsx +++ b/frontend/src/components/ActivityIndicator.tsx @@ -18,6 +18,7 @@ const LABELS: Record = { describe: 'Describing', summarize: 'Summarizing', autotag: 'Suggesting tags', + extract_text: 'Reading text', } function subject(job: ActivityJob): string { diff --git a/frontend/src/components/AppShell.test.tsx b/frontend/src/components/AppShell.test.tsx new file mode 100644 index 0000000..93912c5 --- /dev/null +++ b/frontend/src/components/AppShell.test.tsx @@ -0,0 +1,50 @@ +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { MemoryRouter } from 'react-router-dom' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import AppShell from '@/components/AppShell' +import { useActivityStore } from '@/stores/activity' +import { useAuthStore } from '@/stores/auth' + +beforeEach(() => { + useAuthStore.getState().reset() + // The shell mounts ActivityIndicator, which starts a poll. + vi.spyOn(useActivityStore.getState(), 'start').mockImplementation(() => {}) +}) + +afterEach(() => { + vi.restoreAllMocks() +}) + +function renderShell() { + return render( + + +

Content

+
+
+ ) +} + +describe('AppShell', () => { + it('offers no sign-out to a visitor who is not signed in', () => { + renderShell() + expect(screen.queryByRole('button', { name: /sign out/i })).not.toBeInTheDocument() + }) + + it('signs the user out', async () => { + // `signOut` has existed since M2 and was called by nothing — there was no way out + // of a session short of clearing localStorage by hand. + useAuthStore.setState({ + user: { id: 'u1', username: 'tester' }, + status: 'authenticated', + }) + const signOut = vi.fn() + useAuthStore.setState({ signOut }) + + renderShell() + await userEvent.click(screen.getByRole('button', { name: /sign out/i })) + + expect(signOut).toHaveBeenCalled() + }) +}) diff --git a/frontend/src/components/AppShell.tsx b/frontend/src/components/AppShell.tsx index b36272f..c732251 100644 --- a/frontend/src/components/AppShell.tsx +++ b/frontend/src/components/AppShell.tsx @@ -1,5 +1,5 @@ import type { ReactNode } from 'react' -import { Moon, Search, Settings, Sun } from 'lucide-react' +import { LogOut, Moon, Search, Settings, Sun } from 'lucide-react' import { Link } from 'react-router-dom' import ActivityIndicator from '@/components/ActivityIndicator' import { useAuthStore } from '@/stores/auth' @@ -17,6 +17,7 @@ interface Props { */ export default function AppShell({ children }: Props) { const user = useAuthStore((s) => s.user) + const signOut = useAuthStore((s) => s.signOut) const theme = useThemeStore((s) => s.theme) const toggleTheme = useThemeStore((s) => s.toggle) @@ -54,9 +55,23 @@ export default function AppShell({ children }: Props) { {user && ( - - {user.username} - + <> + + {user.username} + + {/* `signOut` has existed since M2 and was called by nothing, so the only + way out of a session was to clear localStorage by hand. It already + does the whole fan-out — token, four stores, redirect to Notes. */} + + )} diff --git a/frontend/src/components/AssetDetail.tsx b/frontend/src/components/AssetDetail.tsx index 24c0200..bd068a9 100644 --- a/frontend/src/components/AssetDetail.tsx +++ b/frontend/src/components/AssetDetail.tsx @@ -1,5 +1,5 @@ import { useCallback, useEffect, useRef, useState } from 'react' -import { Eye, FileText, Trash2, X } from 'lucide-react' +import { Eye, FileText, Link2, ScanText, Trash2, X } from 'lucide-react' import type { Asset } from '@/api/assets' import { tagsApi } from '@/api/tags' import { enrichmentApi } from '@/api/enrichment' @@ -9,6 +9,7 @@ import { useLibraryStore } from '@/stores/library' import { useTagStore } from '@/stores/tags' import { formatBytes, formatDate, formatDimensions, formatDuration } from '@/utils/format' import AssetThumb from '@/components/AssetThumb' +import DocumentTextPanel from '@/components/DocumentTextPanel' import EmbedButton from '@/components/EmbedButton' import EnrichmentButton from '@/components/EnrichmentButton' import SuggestionPanel from '@/components/SuggestionPanel' @@ -18,6 +19,9 @@ import TranscriptPanel from '@/components/TranscriptPanel' /** Audio and video can be transcribed; nothing else has speech in it. */ const SPEECH_TYPES = new Set(['audio', 'video']) +/** Documents have text read out of them instead — the same idea, a different source. */ +const TEXT_TYPE = 'document' + interface Props { asset: Asset onClose: () => void @@ -50,6 +54,7 @@ export default function AssetDetail({ asset, onClose, startAt }: Props) { const [saving, setSaving] = useState(false) const [tagError, setTagError] = useState(null) const [confirmingDelete, setConfirmingDelete] = useState(false) + const [copied, setCopied] = useState(false) const panelRef = useRef(null) // The detail view owns the player element so the transcript can drive it. Passing a @@ -118,6 +123,23 @@ export default function AssetDetail({ asset, onClose, startAt }: Props) { // What enrichment has cost on this asset. Re-read whenever the asset changes, which // includes after a job finishes — EnrichmentButton refreshes it, and that bumps // `metadata_modified_date`, so this picks up the new spend without its own poll. + // The /a/{id} URL, which is what GN-4 says a Notes document should link to. Built from + // `window.location.origin` rather than a configured base: whichever host the user is + // looking at is the one their colleague can reach too. + const copyLink = useCallback(async () => { + const url = `${window.location.origin}/a/${asset.id}` + try { + await navigator.clipboard.writeText(url) + setCopied(true) + setTimeout(() => setCopied(false), 1500) + } catch { + // Clipboard access needs a secure context and a user gesture, and is refused + // outright in some browsers. Prompting with the URL beats a button that silently + // does nothing. + window.prompt('Copy this link', url) + } + }, [asset.id]) + const [usage, setUsage] = useState(null) useEffect(() => { let current = true @@ -207,14 +229,27 @@ export default function AssetDetail({ asset, onClose, startAt }: Props) {

{asset.name}

- +
+ + +
@@ -236,6 +271,12 @@ export default function AssetDetail({ asset, onClose, startAt }: Props) { />
)} + + {asset.asset_type === TEXT_TYPE && ( +
+ +
+ )}
@@ -302,6 +343,18 @@ export default function AssetDetail({ asset, onClose, startAt }: Props) {
+ {asset.asset_type === TEXT_TYPE && ( + + )} = {}): DocumentPage { + return { + id: 'p1', + idx: 0, + page_number: 1, + label: 'Page 1', + text: 'Gecko Asset Manager', + ...overrides, + } +} + +function job(overrides: Partial = {}): ActivityJob { + return { + id: 'j1', + asset_id: 'a1', + asset_name: 'report.pdf', + action: 'extract_text', + status: 'processing', + stage: 'Extracting text', + progress: 40, + detail: '', + error_message: null, + created_at: '2026-09-16T10:00:00Z', + updated_at: '2026-09-16T10:00:00Z', + ...overrides, + } as ActivityJob +} + +afterEach(() => { + vi.restoreAllMocks() +}) + +describe('DocumentTextPanel', () => { + it('shows the extracted text with its labels', async () => { + vi.spyOn(enrichmentApi, 'text').mockResolvedValue([ + page(), + page({ id: 'p2', idx: 1, page_number: 2, label: 'Page 2', text: 'Second page.' }), + ]) + vi.spyOn(activityApi, 'list').mockResolvedValue([]) + + render() + + expect(await screen.findByText('Gecko Asset Manager')).toBeInTheDocument() + expect(screen.getByText('Page 2')).toBeInTheDocument() + expect(screen.getByText('2 sections')).toBeInTheDocument() + }) + + it('says how to make the document summarisable when nothing has been read', async () => { + vi.spyOn(enrichmentApi, 'text').mockResolvedValue([]) + vi.spyOn(activityApi, 'list').mockResolvedValue([]) + + render() + + expect(await screen.findByText(/run extract text/i)).toBeInTheDocument() + }) + + it('shows only this asset’s extraction, not whatever else is running on it', async () => { + // `activityApi.list` returns every active job for the asset. Taking jobs[0] would + // put a summarise job's progress under this heading — the mistake TranscriptPanel + // already records. + vi.spyOn(enrichmentApi, 'text').mockResolvedValue([]) + vi.spyOn(activityApi, 'list').mockResolvedValue([ + job({ id: 'j2', action: 'summarize', stage: 'Summarising' }), + job(), + ]) + + render() + + expect(await screen.findByText('Extracting text')).toBeInTheDocument() + expect(screen.queryByText('Summarising')).not.toBeInTheDocument() + }) +}) diff --git a/frontend/src/components/DocumentTextPanel.tsx b/frontend/src/components/DocumentTextPanel.tsx new file mode 100644 index 0000000..2134416 --- /dev/null +++ b/frontend/src/components/DocumentTextPanel.tsx @@ -0,0 +1,124 @@ +import { useCallback, useEffect, useState } from 'react' +import { FileText, Loader2 } from 'lucide-react' +import { enrichmentApi, type DocumentPage } from '@/api/enrichment' +import { activityApi, type ActivityJob } from '@/api/transcripts' +import { apiErrorMessage } from '@/api/client' + +interface Props { + assetId: string +} + +/** How often to poll a running extraction. Matches TranscriptPanel: fast enough to feel + * live, slow enough not to hammer the API for a job that takes a while. */ +const POLL_MS = 2000 + +/** + * What extraction read out of this document. + * + * Read-only, unlike `TranscriptPanel`: a transcript is corrected because speech + * recognition mishears, and a person is the authority on what was said. Extracted text + * is a faithful copy of bytes that are already in the file — an edit here would be an + * edit to the document, made in the wrong place. + * + * It exists at all because text nobody can see is indistinguishable from text that was + * never extracted, and this repository has had enough of that. + */ +export default function DocumentTextPanel({ assetId }: Props) { + const [pages, setPages] = useState([]) + const [job, setJob] = useState(null) + const [loading, setLoading] = useState(true) + const [error, setError] = useState(null) + + const load = useCallback(async () => { + try { + const [fetched, jobs] = await Promise.all([ + enrichmentApi.text(assetId), + activityApi.list({ active: true, asset_id: assetId }), + ]) + setPages(fetched) + // Only this asset's extraction. `activityApi.list` returns active jobs of every + // action for the asset, so taking jobs[0] would show a summarise job's progress + // under this heading — the mistake TranscriptPanel already records. + setJob(jobs.find((j) => j.action === 'extract_text') ?? null) + setError(null) + } catch (err) { + setError(apiErrorMessage(err, 'Could not load the extracted text')) + } finally { + setLoading(false) + } + }, [assetId]) + + useEffect(() => { + setLoading(true) + void load() + }, [load]) + + // Poll only while something is running. Extracted text is static once written. + useEffect(() => { + if (!job || (job.status !== 'queued' && job.status !== 'processing')) return + const timer = setInterval(() => void load(), POLL_MS) + return () => clearInterval(timer) + }, [job, load]) + + const running = job?.status === 'queued' || job?.status === 'processing' + + return ( +
+
+ +

+ Extracted text +

+ {pages.length > 0 && ( + + {pages.length} {pages.length === 1 ? 'section' : 'sections'} + + )} + {running && ( + + + {job?.stage || 'Reading…'} + + )} +
+ +
+ {error && ( +

+ {error} +

+ )} + + {!error && loading && ( +

+ Loading… +

+ )} + + {!error && !loading && pages.length === 0 && ( +

+ {running + ? 'Reading the document…' + : 'No text has been read out of this document yet. Run Extract text to make it summarisable.'} +

+ )} + + {!error && + pages.map((page) => ( +
+ {page.label && ( +

+ {page.label} +

+ )} + {/* `whitespace-pre-wrap`: the paragraph breaks extraction preserved are the + only structure this text has left. */} +

+ {page.text} +

+
+ ))} +
+
+ ) +} diff --git a/frontend/src/components/FilterBar.tsx b/frontend/src/components/FilterBar.tsx index eec4fa6..a2fda48 100644 --- a/frontend/src/components/FilterBar.tsx +++ b/frontend/src/components/FilterBar.tsx @@ -1,7 +1,7 @@ import { useEffect, useMemo, useState } from 'react' import { Search, SlidersHorizontal, X } from 'lucide-react' -import type { AssetType } from '@/api/assets' import { buildCategoryTree, type CategoryNode } from '@/api/tags' +import TypeFilterChips, { Chip } from '@/components/TypeFilterChips' import { useLibraryStore } from '@/stores/library' import { useTagStore } from '@/stores/tags' import { formatDuration } from '@/utils/format' @@ -14,14 +14,6 @@ import { formatDuration } from '@/utils/format' * be the reason the grid looks empty. */ -const TYPE_FILTERS: Array<{ value: AssetType | null; label: string }> = [ - { value: null, label: 'All' }, - { value: 'video', label: 'Video' }, - { value: 'image', label: 'Images' }, - { value: 'audio', label: 'Audio' }, - { value: 'document', label: 'Documents' }, -] - /** Mirrors `backend/app/ingest/filetypes.py`; a source the server never writes is noise. */ const SOURCES: Array<{ value: string; label: string }> = [ { value: 'local_upload', label: 'Uploaded' }, @@ -30,30 +22,6 @@ const SOURCES: Array<{ value: string; label: string }> = [ { value: 'gvc_export', label: 'Video Creator' }, ] -function Chip({ - active, - onClick, - children, -}: { - active?: boolean - onClick: () => void - children: React.ReactNode -}) { - return ( - - ) -} - /** An active filter, with the ✕ that clears just it. */ function ActiveChip({ label, onClear }: { label: string; onClear: () => void }) { return ( @@ -192,17 +160,7 @@ export default function FilterBar() { /> -
- {TYPE_FILTERS.map((filter) => ( - setTypeFilter(filter.value)} - > - {filter.label} - - ))} -
+ + + +
+ setNewCategory(e.target.value)} + onKeyDown={(e) => { + if (e.key === 'Enter') void submitCategory() + }} + aria-label="New category" + /> + +
+ + + {loading && tags.length === 0 && ( +

+ Loading… +

+ )} + + {!loading && tags.length === 0 && categories.length === 0 && ( +

+ No tags yet. Tag an asset, or add one above. +

+ )} + +
+ {options.map(({ node, depth }) => ( +
+
+ {editingCategory === node.id ? ( + setDraft(e.target.value)} + onBlur={() => void commitCategoryRename(node)} + onKeyDown={(e) => { + if (e.key === 'Enter') void commitCategoryRename(node) + if (e.key === 'Escape') setEditingCategory(null) + }} + aria-label={`Rename ${node.name}`} + /> + ) : ( + <> +

+ {node.name} +

+ + + + + )} +
+ + +
+ ))} + +
+

+ Unfiled +

+ +
+
+ + ) +} + +interface RowsProps { + tags: Tag[] + options: Array<{ node: CategoryNode; depth: number }> + editingTag: string | null + draft: string + setDraft: (value: string) => void + setEditingTag: (id: string | null) => void + commitRename: (tag: Tag) => Promise + recategorise: (id: string, categoryId: string | null) => Promise + removeTag: (id: string) => Promise +} + +function TagRows({ + tags, + options, + editingTag, + draft, + setDraft, + setEditingTag, + commitRename, + recategorise, + removeTag, +}: RowsProps) { + if (tags.length === 0) { + return

No tags here.

+ } + + return ( +
    + {tags.map((tag) => ( +
  • + {editingTag === tag.id ? ( + <> + setDraft(e.target.value)} + onKeyDown={(e) => { + if (e.key === 'Enter') void commitRename(tag) + if (e.key === 'Escape') setEditingTag(null) + }} + aria-label={`Rename ${tag.name}`} + /> + + + + ) : ( + <> + + {tag.name} + + {typeof tag.asset_count === 'number' && ( + + {tag.asset_count} + + )} + + + + + )} +
  • + ))} +
+ ) +} + +function flatten( + nodes: CategoryNode[], + depth = 0 +): Array<{ node: CategoryNode; depth: number }> { + return nodes.flatMap((node) => [{ node, depth }, ...flatten(node.children, depth + 1)]) +} diff --git a/frontend/src/components/TypeFilterChips.tsx b/frontend/src/components/TypeFilterChips.tsx new file mode 100644 index 0000000..7897691 --- /dev/null +++ b/frontend/src/components/TypeFilterChips.tsx @@ -0,0 +1,62 @@ +import type { AssetType } from '@/api/assets' + +/** + * The asset-type filter, as a row of chips. + * + * Lifted out of `FilterBar` so search can have one too. `FilterBar` itself is not + * reusable here: it is wired to `useLibraryStore` for all nine of its filters, and + * search shares exactly one of them. + */ + +const TYPE_FILTERS: Array<{ value: AssetType | null; label: string }> = [ + { value: null, label: 'All' }, + { value: 'video', label: 'Video' }, + { value: 'image', label: 'Images' }, + { value: 'audio', label: 'Audio' }, + { value: 'document', label: 'Documents' }, +] + +export function Chip({ + active, + onClick, + children, +}: { + active?: boolean + onClick: () => void + children: React.ReactNode +}) { + return ( + + ) +} + +interface Props { + value: AssetType | null + onChange: (value: AssetType | null) => void +} + +export default function TypeFilterChips({ value, onChange }: Props) { + return ( +
+ {TYPE_FILTERS.map((filter) => ( + onChange(filter.value)} + > + {filter.label} + + ))} +
+ ) +} diff --git a/frontend/src/stores/library.ts b/frontend/src/stores/library.ts index e451a26..3d991fe 100644 --- a/frontend/src/stores/library.ts +++ b/frontend/src/stores/library.ts @@ -70,6 +70,7 @@ interface LibraryState { /** Re-read one asset from the server, for when something other than the user * changed it — an enrichment job writing a summary, for instance. */ refreshAsset: (id: string) => Promise + openById: (id: string) => Promise applyTags: (assetIds: string[], add: string[], remove: string[]) => Promise dismissRejections: () => void reset: () => void @@ -281,6 +282,23 @@ export const useLibraryStore = create((set, get) => ({ } }, + async openById(id) { + // Fetches one asset *and puts it in `assets`*, which is the whole point. `update`, + // `remove`, `setAssetTags` and `refreshAsset` all work by mapping over that list, so + // a detail view rendered for an asset that is not in it would show working controls + // whose every write silently landed nowhere. + // + // No staleness token: this is keyed to a route parameter, so a second call means the + // user navigated to a different asset and its result is the one that should win. + const fresh = await assetsApi.get(id) + set((state) => ({ + assets: state.assets.some((a) => a.id === id) + ? state.assets.map((a) => (a.id === id ? fresh : a)) + : [fresh, ...state.assets], + })) + return fresh + }, + async refreshAsset(id) { // No optimistic step and no error surfaced: this runs after a background job, not // after something the user did, so a failure here should leave the panel showing diff --git a/frontend/src/stores/tags.test.ts b/frontend/src/stores/tags.test.ts new file mode 100644 index 0000000..02cd9ca --- /dev/null +++ b/frontend/src/stores/tags.test.ts @@ -0,0 +1,100 @@ +import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest' +import { tagsApi, type Tag, type TagCategory } from '@/api/tags' +import { useTagStore } from '@/stores/tags' + +function tag(overrides: Partial = {}): Tag { + return { id: 't1', name: 'interview', category_id: null, ...overrides } +} + +function category(overrides: Partial = {}): TagCategory { + return { id: 'c1', name: 'Formats', parent_category_id: null, ...overrides } +} + +beforeEach(() => { + useTagStore.getState().reset() +}) + +afterEach(() => { + vi.restoreAllMocks() +}) + +describe('useTagStore', () => { + it('creates a tag and folds it into the catalogue', async () => { + vi.spyOn(tagsApi, 'create').mockResolvedValue(tag({ id: 't9', name: 'neuroweapons' })) + + await useTagStore.getState().create('neuroweapons', null) + + expect(useTagStore.getState().tags.map((t) => t.name)).toEqual(['neuroweapons']) + }) + + it('does not duplicate a tag the get-or-create endpoint handed back', async () => { + useTagStore.setState({ tags: [tag()] }) + vi.spyOn(tagsApi, 'create').mockResolvedValue(tag()) + + await useTagStore.getState().create('Interview', null) + + expect(useTagStore.getState().tags).toHaveLength(1) + }) + + it('puts an optimistic rename back when the name is taken', async () => { + useTagStore.setState({ tags: [tag()] }) + vi.spyOn(tagsApi, 'rename').mockRejectedValue(new Error('taken')) + + await expect(useTagStore.getState().rename('t1', 'talk')).rejects.toThrow() + + expect(useTagStore.getState().tags[0].name).toBe('interview') + expect(useTagStore.getState().error).toBeTruthy() + }) + + it('moves a tag into a category', async () => { + useTagStore.setState({ tags: [tag()] }) + vi.spyOn(tagsApi, 'recategorise').mockResolvedValue(tag({ category_id: 'c1' })) + + await useTagStore.getState().recategorise('t1', 'c1') + + expect(useTagStore.getState().tags[0].category_id).toBe('c1') + }) + + it('puts a refused category move back, so the tree never renders a cycle', async () => { + useTagStore.setState({ + categories: [category(), category({ id: 'c2', name: 'Talks' })], + }) + vi.spyOn(tagsApi, 'updateCategory').mockRejectedValue(new Error('cycle')) + + await expect( + useTagStore.getState().updateCategory('c1', { parent_category_id: 'c2' }) + ).rejects.toThrow() + + expect(useTagStore.getState().categories[0].parent_category_id).toBeNull() + }) + + it('reloads after deleting a category, because the server lifts its children', async () => { + useTagStore.setState({ categories: [category()] }) + vi.spyOn(tagsApi, 'removeCategory').mockResolvedValue(undefined) + const list = vi.spyOn(tagsApi, 'list').mockResolvedValue([]) + vi.spyOn(tagsApi, 'listCategories').mockResolvedValue([]) + + await useTagStore.getState().removeCategory('c1') + + expect(list).toHaveBeenCalled() + }) + + it('does not repopulate a store that was reset while a request was in flight', async () => { + // Sign-out empties the store. A rejection landing afterwards used to put the + // previous user's vocabulary back into it. + useTagStore.setState({ tags: [tag()] }) + let reject: (reason: Error) => void = () => {} + vi.spyOn(tagsApi, 'remove').mockReturnValue( + new Promise((_, r) => { + reject = r + }) + ) + + const pending = useTagStore.getState().remove('t1') + useTagStore.getState().reset() + reject(new Error('gone')) + await pending + + expect(useTagStore.getState().tags).toEqual([]) + }) +}) diff --git a/frontend/src/stores/tags.ts b/frontend/src/stores/tags.ts index b164a19..5bc9f4a 100644 --- a/frontend/src/stores/tags.ts +++ b/frontend/src/stores/tags.ts @@ -31,9 +31,16 @@ interface TagState { ensureLoaded: () => void /** Fold a tag the server just returned into the catalogue, without a refetch. */ remember: (tags: Tag[]) => void + create: (name: string, categoryId?: string | null) => Promise rename: (id: string, name: string) => Promise + /** Move a tag into a category, or out of every category with null. */ + recategorise: (id: string, categoryId: string | null) => Promise remove: (id: string) => Promise createCategory: (name: string, parentId?: string | null) => Promise + updateCategory: ( + id: string, + changes: { name?: string; parent_category_id?: string | null } + ) => Promise removeCategory: (id: string) => Promise reset: () => void } @@ -84,46 +91,115 @@ export const useTagStore = create((set, get) => ({ }) }, + async create(name, categoryId) { + const token = requestToken + try { + const created = await tagsApi.create(name, categoryId) + if (token !== requestToken) return + // `POST /tags` is get-or-create, so this can return a tag that already exists. + // `remember` merges by id rather than appending, which is exactly right for that. + get().remember([created]) + } catch (error) { + if (token !== requestToken) return + set({ error: apiErrorMessage(error, 'Could not create that tag') }) + throw error + } + }, + async rename(id, name) { + const token = requestToken const previous = get().tags set((state) => ({ tags: state.tags.map((t) => (t.id === id ? { ...t, name } : t)), })) try { const saved = await tagsApi.rename(id, name) + if (token !== requestToken) return set((state) => ({ tags: state.tags.map((t) => (t.id === id ? { ...t, ...saved } : t)), })) } catch (error) { // A rename can be refused — the name may already be taken, case-insensitively — - // so the optimistic edit has to come back out. + // so the optimistic edit has to come back out. Guarded: a rejection landing after + // `reset()` would otherwise put the previous user's vocabulary back into a store + // that was emptied on sign-out. + if (token !== requestToken) return set({ tags: previous, error: apiErrorMessage(error, 'Could not rename that tag') }) throw error } }, + async recategorise(id, categoryId) { + const token = requestToken + const previous = get().tags + set((state) => ({ + tags: state.tags.map((t) => (t.id === id ? { ...t, category_id: categoryId } : t)), + })) + try { + const saved = await tagsApi.recategorise(id, categoryId) + if (token !== requestToken) return + set((state) => ({ + tags: state.tags.map((t) => (t.id === id ? { ...t, ...saved } : t)), + })) + } catch (error) { + if (token !== requestToken) return + set({ tags: previous, error: apiErrorMessage(error, 'Could not move that tag') }) + throw error + } + }, + async remove(id) { + const token = requestToken const previous = get().tags set((state) => ({ tags: state.tags.filter((t) => t.id !== id) })) try { await tagsApi.remove(id) } catch (error) { + if (token !== requestToken) return set({ tags: previous, error: apiErrorMessage(error, 'Could not delete that tag') }) throw error } }, async createCategory(name, parentId) { + const token = requestToken try { const created = await tagsApi.createCategory(name, parentId) + if (token !== requestToken) return set((state) => ({ categories: [...state.categories, created] })) } catch (error) { + if (token !== requestToken) return set({ error: apiErrorMessage(error, 'Could not create that category') }) throw error } }, + async updateCategory(id, changes) { + const token = requestToken + const previous = get().categories + set((state) => ({ + categories: state.categories.map((c) => (c.id === id ? { ...c, ...changes } : c)), + })) + try { + const saved = await tagsApi.updateCategory(id, changes) + if (token !== requestToken) return + set((state) => ({ + categories: state.categories.map((c) => (c.id === id ? { ...c, ...saved } : c)), + })) + } catch (error) { + // The server refuses a move that would put a category inside itself. The + // optimistic edit has to come out, or the tree renders a cycle the API rejected. + if (token !== requestToken) return + set({ + categories: previous, + error: apiErrorMessage(error, 'Could not update that category'), + }) + throw error + } + }, + async removeCategory(id) { + const token = requestToken const previous = get().categories set((state) => ({ categories: state.categories.filter((c) => c.id !== id) })) try { @@ -132,6 +208,7 @@ export const useTagStore = create((set, get) => ({ // cascading, so both lists are now stale in a way this store cannot derive. void get().load() } catch (error) { + if (token !== requestToken) return set({ categories: previous, error: apiErrorMessage(error, 'Could not delete that category'), diff --git a/frontend/src/views/AssetView.test.tsx b/frontend/src/views/AssetView.test.tsx new file mode 100644 index 0000000..d4ee881 --- /dev/null +++ b/frontend/src/views/AssetView.test.tsx @@ -0,0 +1,133 @@ +import { render, screen } from '@testing-library/react' +import { MemoryRouter, Route, Routes } from 'react-router-dom' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { AxiosError, AxiosHeaders } from 'axios' +import AssetView from '@/views/AssetView' +import { assetsApi, type Asset } from '@/api/assets' +import { tagsApi } from '@/api/tags' +import { usageApi } from '@/api/usage' +import { useLibraryStore } from '@/stores/library' +import { useTagStore } from '@/stores/tags' + +function asset(overrides: Partial = {}): Asset { + return { + id: 'a1', + name: 'Giordano interview', + description: null, + summary: null, + asset_type: 'video', + source: 'local_upload', + original_name: 'giordano.mp4', + mime_type: 'video/mp4', + file_format: 'mp4', + size_bytes: 100, + duration_seconds: 3600, + width: 1920, + height: 1080, + codec: 'h264', + file_url: '/media/u/a1.mp4?exp=1&sig=x', + thumb_url: '/media/u/a1.thumb.jpg?exp=1&sig=x', + missing: false, + tags: [], + upload_date: '2026-09-13T10:00:00Z', + modified_date: '2026-09-13T10:00:00Z', + metadata_modified_date: '2026-09-13T10:00:00Z', + ...overrides, + } +} + +function notFound(): AxiosError { + const error = new AxiosError('Not Found') + error.response = { + status: 404, + statusText: 'Not Found', + data: { detail: { code: 'not_found', message: 'No such asset' } }, + headers: {}, + config: { headers: new AxiosHeaders() }, + } + return error +} + +function renderAt(path: string) { + return render( + + + } /> + The library

} /> +
+
+ ) +} + +beforeEach(() => { + useLibraryStore.getState().reset() + useTagStore.getState().reset() + // AssetDetail calls `ensureLoaded` — it is reachable from search, where no FilterBar + // has loaded the catalogue. Stubbed so the suite makes no real request. + vi.spyOn(tagsApi, 'list').mockResolvedValue([]) + vi.spyOn(tagsApi, 'listCategories').mockResolvedValue([]) + vi.spyOn(usageApi, 'forAsset').mockResolvedValue({ + total_cost: 0, + events: 0, + estimated: false, + } as never) +}) + +afterEach(() => { + vi.restoreAllMocks() +}) + +describe('AssetView', () => { + it('loads an asset that was never in the library list', async () => { + // The point of the route: a link from Notes lands here with an empty store. + vi.spyOn(assetsApi, 'get').mockResolvedValue(asset()) + + renderAt('/a/a1') + + expect( + await screen.findByRole('dialog', { name: 'Giordano interview' }) + ).toBeInTheDocument() + }) + + it('puts the asset in the library store, so the panel controls write somewhere', async () => { + // `update`, `remove` and `setAssetTags` all map over `assets`. An asset missing + // from that list gets working-looking controls whose every write lands nowhere. + vi.spyOn(assetsApi, 'get').mockResolvedValue(asset()) + + renderAt('/a/a1') + await screen.findByRole('dialog', { name: 'Giordano interview' }) + + expect(useLibraryStore.getState().assets.map((a) => a.id)).toEqual(['a1']) + }) + + it('says so when the asset is gone, rather than redirecting silently', async () => { + vi.spyOn(assetsApi, 'get').mockRejectedValue(notFound()) + + renderAt('/a/missing') + + expect(await screen.findByText(/that asset is not here/i)).toBeInTheDocument() + }) + + it('reads a timestamp off the URL', async () => { + const spy = vi.spyOn(assetsApi, 'get').mockResolvedValue(asset()) + + renderAt('/a/a1?t=412.5') + await screen.findByRole('dialog', { name: 'Giordano interview' }) + + expect(spy).toHaveBeenCalledWith('a1') + // The player seeks on `loadedmetadata`, which jsdom never fires, so the assertion + // that matters here is that a malformed value cannot reach it — see below. + }) + + it('ignores a timestamp that is not a number', async () => { + vi.spyOn(assetsApi, 'get').mockResolvedValue(asset()) + + renderAt('/a/a1?t=banana') + + // Nothing to assert on the player, but rendering at all proves NaN did not reach + // `currentTime`, which throws in a real browser. + expect( + await screen.findByRole('dialog', { name: 'Giordano interview' }) + ).toBeInTheDocument() + }) +}) diff --git a/frontend/src/views/AssetView.tsx b/frontend/src/views/AssetView.tsx new file mode 100644 index 0000000..8bd6311 --- /dev/null +++ b/frontend/src/views/AssetView.tsx @@ -0,0 +1,102 @@ +import { useEffect, useState } from 'react' +import { useNavigate, useParams, useSearchParams } from 'react-router-dom' +import { FileQuestion } from 'lucide-react' +import { apiErrorCode, apiErrorMessage } from '@/api/client' +import { useLibraryStore } from '@/stores/library' +import AssetDetail from '@/components/AssetDetail' + +/** + * One asset, at its own URL. + * + * `docs/gecko-notes-integration.md` GN-4 settles the Notes→GAM asset reference as a + * plain link to `/a/{assetId}`, and chose that over a custom BlockNote block partly + * *because* it needed no work in Notes. That reasoning assumed this end existed; it did + * not, and until now `/a/anything` fell through the catch-all route to the library with + * no sign anything had gone wrong. + * + * Renders the same `AssetDetail` the library modal does, reading from the library store + * so its edit, delete and tag controls write to the same place they always have. + */ +export default function AssetView() { + const { id = '' } = useParams() + const [params] = useSearchParams() + const navigate = useNavigate() + const openById = useLibraryStore((s) => s.openById) + const asset = useLibraryStore((s) => s.assets.find((a) => a.id === id)) + const [loading, setLoading] = useState(!asset) + const [missing, setMissing] = useState(false) + const [error, setError] = useState(null) + + useEffect(() => { + let current = true + setMissing(false) + setError(null) + setLoading(true) + openById(id) + .then(() => { + if (current) setLoading(false) + }) + .catch((err: unknown) => { + if (!current) return + setLoading(false) + if (apiErrorCode(err) === 'not_found') setMissing(true) + else setError(apiErrorMessage(err, 'Could not load this asset')) + }) + return () => { + current = false + } + }, [id, openById]) + + // A search hit deep-links to the moment it matched: /a/{id}?t=412.0. `Number('')` is + // 0, not NaN, so the presence check has to come first — otherwise every link without + // a timestamp would seek to the start, which looks like a bug on a video the user had + // already scrubbed. + const raw = params.get('t') + const parsed = raw === null ? Number.NaN : Number(raw) + const startAt = Number.isFinite(parsed) && parsed >= 0 ? parsed : undefined + + if (loading && !asset) { + return ( +

+ Loading… +

+ ) + } + + if (missing || (!asset && !error)) { + return navigate('/library')} /> + } + + if (error && !asset) { + return ( +
+

+ {error} +

+
+ ) + } + + if (!asset) return null + + return ( + navigate('/library')} /> + ) +} + +function NotFound({ onBack }: { onBack: () => void }) { + return ( +
+ +

+ That asset is not here +

+

+ It may have been deleted, or the link may belong to someone else's library. +

+ +
+ ) +} diff --git a/frontend/src/views/SearchView.test.tsx b/frontend/src/views/SearchView.test.tsx index ef4a43c..fb4d62f 100644 --- a/frontend/src/views/SearchView.test.tsx +++ b/frontend/src/views/SearchView.test.tsx @@ -167,3 +167,57 @@ describe('SearchView', () => { expect(run.mock.calls.length).toBeLessThan(4) }) }) + +describe('SearchView filters and paging', () => { + /** The params the last search was actually run with — the gap these tests exist for + * is that `searchApi.run` typed both of these and the view passed neither. */ + function lastParams(spy: { mock: { calls: unknown[][] } }) { + const calls = spy.mock.calls + return calls[calls.length - 1]?.[1] + } + + it('sends the server default limit with an unfiltered search', async () => { + const spy = vi.spyOn(searchApi, 'run').mockResolvedValue(response()) + renderView() + + await userEvent.type(screen.getByPlaceholderText(/what are you looking for/i), 'nano') + await screen.findByText('6:52') + + expect(lastParams(spy)).toMatchObject({ limit: 30 }) + expect(lastParams(spy)).not.toHaveProperty('asset_type') + }) + + it('sends the chosen asset type', async () => { + const spy = vi.spyOn(searchApi, 'run').mockResolvedValue(response()) + renderView() + + await userEvent.type(screen.getByPlaceholderText(/what are you looking for/i), 'nano') + await screen.findByText('6:52') + await userEvent.click(screen.getByRole('button', { name: 'Documents' })) + + await waitFor(() => expect(lastParams(spy)).toMatchObject({ asset_type: 'document' })) + }) + + it('asks for a bigger page rather than an offset, because there is no offset', async () => { + const spy = vi + .spyOn(searchApi, 'run') + .mockResolvedValue(response({ data: Array.from({ length: 30 }, () => hit()) })) + renderView() + + await userEvent.type(screen.getByPlaceholderText(/what are you looking for/i), 'nano') + await screen.findByRole('button', { name: /show more/i }) + await userEvent.click(screen.getByRole('button', { name: /show more/i })) + + await waitFor(() => expect(lastParams(spy)).toMatchObject({ limit: 60 })) + }) + + it('offers no "show more" when the page is not full', async () => { + vi.spyOn(searchApi, 'run').mockResolvedValue(response()) + renderView() + + await userEvent.type(screen.getByPlaceholderText(/what are you looking for/i), 'nano') + await screen.findByText('6:52') + + expect(screen.queryByRole('button', { name: /show more/i })).not.toBeInTheDocument() + }) +}) diff --git a/frontend/src/views/SearchView.tsx b/frontend/src/views/SearchView.tsx index cf7b907..81987c7 100644 --- a/frontend/src/views/SearchView.tsx +++ b/frontend/src/views/SearchView.tsx @@ -5,16 +5,29 @@ import { searchApi, splitHighlights, type SearchHit } from '@/api/search' import { apiErrorMessage } from '@/api/client' import AssetDetail from '@/components/AssetDetail' import AssetThumb from '@/components/AssetThumb' +import TypeFilterChips from '@/components/TypeFilterChips' import { formatDuration } from '@/utils/format' -import type { Asset } from '@/api/assets' +import type { Asset, AssetType } from '@/api/assets' /** Long enough that typing does not fire a request per keystroke, short enough that * results feel like they are keeping up. */ const DEBOUNCE_MS = 250 +/** The server's default, and its cap (`le=100` in routers/search.py). There is no + * `offset` on that endpoint and `total` is the size of the page rather than of the + * corpus, so "show more" asks for a bigger page — it is not a pager, and cannot be one + * without a backend change. */ +const PAGE_SIZE = 30 +const MAX_LIMIT = 100 + export default function SearchView() { const [params, setParams] = useSearchParams() const query = params.get('q') ?? '' + const typeParam = params.get('type') + const assetType = ( + typeParam && typeParam !== 'all' ? typeParam : null + ) as AssetType | null + const limit = clampLimit(params.get('limit')) const [draft, setDraft] = useState(query) const [hits, setHits] = useState([]) @@ -31,44 +44,62 @@ export default function SearchView() { // the results the user is reading. const token = useRef(0) - const run = useCallback(async (q: string) => { - const mine = ++token.current - if (!q.trim()) { - setHits([]) - setSearched(false) - return - } - - setLoading(true) - try { - const response = await searchApi.run(q) - if (mine !== token.current) return - setHits(response.data) - setSemantic(response.semantic) - setSemanticError(response.semantic_error) - setError(null) - setSearched(true) - } catch (err) { - if (mine !== token.current) return - setError(apiErrorMessage(err, 'Search failed')) - } finally { - if (mine === token.current) setLoading(false) - } - }, []) - - // Keep the URL in step so a search can be linked to or reloaded. + const run = useCallback( + async (q: string, options: { assetType: AssetType | null; limit: number }) => { + const mine = ++token.current + if (!q.trim()) { + setHits([]) + setSearched(false) + return + } + + setLoading(true) + try { + const response = await searchApi.run(q, { + ...(options.assetType ? { asset_type: options.assetType } : {}), + limit: options.limit, + }) + if (mine !== token.current) return + setHits(response.data) + setSemantic(response.semantic) + setSemanticError(response.semantic_error) + setError(null) + setSearched(true) + } catch (err) { + if (mine !== token.current) return + setError(apiErrorMessage(err, 'Search failed')) + } finally { + if (mine === token.current) setLoading(false) + } + }, + [] + ) + + // Keep the URL in step so a search can be linked to or reloaded — including the type + // filter and how many results are being shown, so a reload does not silently shrink + // the list somebody had expanded. useEffect(() => { const timer = setTimeout(() => { if (draft !== query) { - setParams(draft.trim() ? { q: draft } : {}, { replace: true }) + setParams(nextParams(draft, assetType, limit), { replace: true }) } - void run(draft) + void run(draft, { assetType, limit }) }, DEBOUNCE_MS) return () => clearTimeout(timer) // `query` deliberately omitted: including it would re-run on the URL update this // effect itself causes. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [draft, run, setParams]) + }, [draft, run, setParams, assetType, limit]) + + // Changing the filter or asking for more resets the page size question rather than + // waiting out the debounce: neither is a keystroke, so there is nothing to debounce. + const applyType = (value: AssetType | null) => { + setParams(nextParams(draft, value, PAGE_SIZE)) + } + + const showMore = () => { + setParams(nextParams(draft, assetType, Math.min(limit + PAGE_SIZE, MAX_LIMIT))) + } return (
@@ -83,6 +114,8 @@ export default function SearchView() { />
+ +

Searches names, descriptions and every spoken word.{' '} {semantic ? ( @@ -145,6 +178,14 @@ export default function SearchView() { )} + {hits.length >= limit && limit < MAX_LIMIT && ( +

+ +
+ )} + {open && ( { + const next: Record = {} + if (q.trim()) next.q = q + if (assetType) next.type = assetType + if (limit !== PAGE_SIZE) next.limit = String(limit) + return next +} + function ResultRow({ hit, onOpen }: { hit: SearchHit; onOpen: () => void }) { const parts = splitHighlights(hit.snippet) const timestamp = hit.start_time !== null ? formatDuration(hit.start_time) : '' diff --git a/frontend/src/views/SettingsView.tsx b/frontend/src/views/SettingsView.tsx index cfa18f7..c64df93 100644 --- a/frontend/src/views/SettingsView.tsx +++ b/frontend/src/views/SettingsView.tsx @@ -11,6 +11,7 @@ import { apiErrorMessage } from '@/api/client' import { embeddingsApi, type EmbeddingCoverage } from '@/api/embeddings' import { isActive, useActivityStore } from '@/stores/activity' import ProviderPanel from '@/components/ProviderPanel' +import TagPanel from '@/components/TagPanel' import UsagePanel from '@/components/UsagePanel' import { useSavedFlash } from '@/utils/useSavedFlash' @@ -509,6 +510,7 @@ export default function SettingsView() { + ) }