feat: M10 — attribution - #29
Merged
Merged
Conversation
…e index
M10, steps 1 and 2 of six. Adds the eight attribution columns, the module
that composes and inherits them, and the index rebuild that makes them
findable. Nothing writes them yet — that is the harvester, next.
`published_date` is a string against this repo's own datetime convention,
and the model comment says why so it does not get "fixed": publication
dates are routinely partial ("1994", "March 2019") and a datetime cannot
hold either without inventing a January 1st that then reads as real. ISO
partial dates compare correctly as plain strings, so the date filters
need no parsing.
`credit_line` is an override, not the composed value. Storing the
composition would leave it stale the moment `publisher` is corrected —
the same rot that made copy-on-create wrong for a clip's inherited
attribution one level down.
Two migrations, not one. The second drops and recreates `asset_fts`,
because FTS5 has no ALTER TABLE ADD COLUMN — and since that table stores
its own copy of the text rather than using external-content mode, the
recreate destroys the index for every existing asset. The repopulate is
what puts it back, and it is invisible to any test that only compares
columns: without it the schema is perfect and keyword search silently
returns nothing until each asset happens to be edited again. Splitting it
out means a failure in that half cannot strand the columns from the
first. Verified by removing the repopulate and watching the new tests go
red.
Inheritance resolves through `parent_asset_id` on read, one level — which
is complete, not a simplification, since `create_clip` refuses to clip a
clip and `promote_clip` keeps the pointer on the original. The keyword
index is the one place that cannot resolve on read, because it stores a
snapshot, so `_apply` re-indexes an asset's children when an attribution
field changes. Without that, correcting a publisher would leave every
clip of it indexed under the old one.
Also folds the two existing metadata writers into a shared `_apply` that
takes the provenance stamp as an argument, and adds a third value,
"embedded", beside "human" and "ai". A tag read out of a file is a fact
about the file but not a claim anyone checked — often the camera owner or
a studio default — and keeping it distinct is what will let a later pass
propose over it while never proposing over something the user typed.
858 backend tests pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019RWM7S6z1UPsZqso4p2HAj
M10, steps 3 and 4 of six. Attribution is now written at ingest, editable by hand, resolved through a clip's parent on read, filterable, and searchable. The harvest reads no new data off the wire. `ingest/probe.py` has always run ffprobe with `-show_format`, which returns the container's tag block, and GAM parsed out the duration and discarded the rest. `ProbeResult` carries it now. EXIF/IPTC, PDF Info and Office core properties join it. Four things the harvester deliberately does not do, each of which was a choice rather than an omission: - It never maps a media container's `title`. There it names the file, not a containing work, and it is very often an encoder's boilerplate; the video fixture sets one precisely so a test can assert it goes nowhere. `album`/`show` do name a work, so those map. Documents go the other way — a document's title is the work's own — and the asymmetry is commented where it would otherwise look arbitrary. - It reads only an allowlist of tag keys. Every MP4 carries `encoder`, `handler_name` and `major_brand`; a mapping that took whatever it recognised would file "Lavf60.16.100" as somebody's creator. - It never harvests `retrieved_at`. When you fetched something is not a fact the file can know. - It only fills blanks. That is what makes it safe to run unasked, and safe for the library-wide re-harvest to run twice. Two bugs caught by testing against real files rather than stubbed tag dictionaries, both of which would have passed against a stub: DateTimeOriginal lives in the Exif sub-IFD rather than IFD0, so reading only the top level works on hand-built fixtures and fails on every actual photograph; and PDF writes its date as "D:20190315101112Z", prefixed and separator-less, which the normaliser did not match. The fixture generator carries the same trap — Pillow serialises the sub-IFD from the value stored under 0x8769, so mutating what get_ifd() returns is silently dropped on save. Filters resolve through the parent the same way the read model does. A filter that only looked at the row would answer "everything from the BBC" with the documentary and none of the clips cut from it, which reads as a bug and is the kind of incompleteness a user cannot notice. `unattributed` follows the same rule: a clip showing its parent's credit is not missing one, and counting it would put rows in the backlog there is nothing to do about. `published_date` accepts only YYYY, YYYY-MM or YYYY-MM-DD. The string column is what allows a partial date to exist; the validator is what stops it becoming free text, where "summer 1994" would sort meaninglessly and break range filters that compare lexicographically precisely because the format is guaranteed. 916 backend tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RWM7S6z1UPsZqso4p2HAj
M10, step 5 of six. Attribution is now reachable: a Source tab on the asset panel, creator/publisher filters and a "missing a source" chip in the library, and the resolved credit in the Info facts. The panel renders resolved values, and marks the inherited ones. That marking is load-bearing rather than decorative: an inherited value that looked typed is one a user would "correct" in place, which silently detaches that field from its source so that fixing the original later no longer reaches it. The clip is told where the value came from instead. Saving sends only the fields that changed, for the reason the description editor already does: the API stamps every key it receives as human-written, so resending the seven you did not touch would mark them hand-verified — including any a harvest had filled, which is exactly the distinction the "embedded" provenance value exists to keep. The credit preview duplicates the server's composition so it updates as you type. Deliberate: the alternative is a round trip per keystroke to render pure formatting, and the server's `credit` stays authoritative for anything saved. `unattributed` is only sent when the chip is on. `false` is a real filter server-side — "everything that *has* attribution" — so sending it whenever the chip was off would hide every unattributed asset from the default library view, which is the opposite of the point. The filter inputs commit on blur or Enter rather than per keystroke: every filter change reloads the library, so a controlled input bound straight to the store would be one request per character. Escape is stopped at the panel. `TagInput` and the transcript editor let it through to `DetailDock`'s window listener, so abandoning a half-typed value there closes the whole asset; that bug is not in this milestone's scope, but this is not a ninth instance of it. Six existing test files build their own asset literals, so the ten new fields land via one shared `noAttribution` spread rather than being typed out six times. 280 frontend tests, 916 backend. Lint and build clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RWM7S6z1UPsZqso4p2HAj
M10, step 6 of six — the milestone is complete. The one enrichment that may never write directly, and the asymmetry is the whole design. A description the model gets wrong is a poorer search result. A citation it gets wrong credits somebody else's work to the wrong outlet, in a field that then looks finished — a blank publisher prompts a fix, a confidently wrong one does not. So this produces Suggestion rows and stops, the shape FR 9.1.4 already gives autotag. The grounding rule is the other half, and it is enforced rather than requested: the model must quote the thing it read each field from — a chyron, a byline, a title page, a watermark — and a field arriving with no evidence is dropped by the parser regardless of what the prompt asked for. A model that returns a publisher without evidence has guessed. The quote is stored on the suggestion and shown in the panel, because a reviewer who cannot see what the model read is not reviewing anything. "Nothing it could point to" is a correct and common answer. Accepting writes through `apply_metadata`, the human path, stamping provenance "human" — the user read the evidence and chose it, exactly as an accepted title already works. A field that already has a value is never proposed over, since otherwise "accept" would become a way to quietly overwrite one. One suggestion kind carrying JSON, not eight kinds: `accept` dispatches on kind, and eight near-identical branches would be eight places to forget one. Still one row per field, so the publisher can be taken and the date declined — which is the common case, since a model reads a channel logo far more reliably than a broadcast date. `propose` and `propose_attribution` now each clear only their own pending rows. Clearing every pending row, as `propose` did, would have thrown away suggestions from a run it knows nothing about and the user has not seen. Two things found while wiring the UI. `Decide` built its accept button's accessible name from `Suggestion.value`, which for an attribution row is the JSON payload — a screen reader would have read out a blob; it uses the decoded value now. And `BulkAction` gained 'attribute' on the frontend before the backend's ACTIONS map had it, which would have been a 400 from a button that looked fine: rather than removing it, the bulk pass the spec left undecided is built, since it turned out to be one map entry and one button over machinery M6 already put in place. It writes nothing either way, so it carries none of the risk that keeps transcription off that list. 945 backend tests, 283 frontend. Lint, format and build clean. Docs updated: M10 marked complete, with the resolved open question and the two real-file bugs recorded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RWM7S6z1UPsZqso4p2HAj
davior
marked this pull request as ready for review
September 20, 2026 09:10
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds the spec merged in #28. An asset can now say whose work it is, not just how the file arrived.
Four commits, in dependency order: schema and composition → harvester and API → the Source tab and filters → AI suggestions.
What works now
The decisions worth reviewing
Embedded metadata is written; an AI may only suggest. EXIF
Artistis a fact about the file. What a model reads off a chyron is a claim. The failure modes are not symmetric: a blank publisher is visibly incomplete and prompts a fix, while a confidently wrong one looks finished and ends up crediting somebody else's work to the wrong outlet. So the AI path producesSuggestionrows and stops.The grounding rule is enforced in the parser, not just asked for in the prompt. The model must quote the thing it read each field from; a field arriving with no evidence is dropped regardless of what it was told. The quote is stored and shown in the panel, because a reviewer who cannot see what the model read is not reviewing anything.
"Nothing it could point to"is a correct and common answer.Clips inherit at read time rather than copying at creation. Copying rots: correcting a publisher six months on would never reach the clips already cut from it, silently. Resolution costs no extra query —
to_read_modelalready receives the parent, batch-loaded since M7.Filters follow inheritance too. A filter that only looked at the row would answer "everything from the BBC" with the documentary and none of its clips — a quiet incompleteness the user has no way to notice.
unattributedfollows the same rule: a clip showing its parent's credit is not missing one.Two schema choices that break house convention on purpose
Both are commented where they would otherwise look like mistakes:
published_dateis a string, not adatetime. Publication dates are routinely partial — a book is from 1994. Adatetimeforces a fabricated January 1st indistinguishable from a real one, which in a citation record is exactly the quiet falsehood this milestone exists to prevent. ISO partial dates compare correctly as strings, so the range filters need no parsing; a validator keeps the format guaranteed.credit_lineis an override, not the composed value. A stored composition is stale the moment a component field is corrected.field_provenancegains a third value,"embedded": a tag read out of a file is a fact about the file but not a claim anyone checked (it is frequently the camera's registered owner), and keeping it distinct from"human"is what lets a later pass propose over it while never proposing over something you typed.The migration risk, and how it is handled
SQLite FTS5 has no
ALTER TABLE ADD COLUMN, so indexing attribution means dropping and recreatingasset_fts— and because that table stores its own copy of the text rather than using external-content mode, the recreate destroys the index for every existing asset. The repopulate is invisible to any test that only compares columns: without it the schema is perfect and keyword search silently returns nothing until each asset happens to be edited again.It gets its own Alembic revision, separate from the eight
add_columncalls, so a failure there cannot strand the columns. Verified by removing the repopulate and watching the new tests go red.One related subtlety: the keyword index is the only place inheritance cannot resolve on read, because it stores a snapshot — so an attribution change re-indexes the asset's children. Without that, correcting a parent's publisher leaves every clip of it indexed under a value no longer shown anywhere.
Bugs real files caught
The fixtures are real files rather than mocked tag dictionaries, and two things would have passed against a stub:
DateTimeOriginallives in the Exif sub-IFD (0x8769), not IFD0. Reading only the top level works on hand-built fixtures and fails on every actual photograph. The fixture generator carries the matching trap: Pillow serialises the sub-IFD from the value stored under 0x8769, so mutating the dictget_ifd()returns is silently dropped on save.D:20190315101112Z— prefixed and separator-less, unlike every other format.Also found while wiring the UI:
Decidebuilt its accept button's accessible name fromSuggestion.value, which for an attribution row is the JSON payload — a screen reader would have read out a blob.Resolved from the spec
The spec left "a bulk attribution pass over a selection" genuinely undecided. Built — it turned out to be one entry in
bulk.py::ACTIONSand one button, over machinery M6 already put in place, well under the cost of the design discussion the question implied.Testing
945backend tests (up from 916),283frontend (up from 264). Lint, Prettier and build clean.Note for anyone running this locally: it adds two migrations, and migrations do not run outside Docker —
alembic upgrade headbefore starting the backend.Not covered by tests, and the one thing worth trying by hand: the AI suggestion path needs a real provider configured in Settings. Every test here drives it through a stubbed upstream, which proves the grounding rule and the parser and says nothing about how often a real model can actually ground a field.
🤖 Generated with Claude Code
https://claude.ai/code/session_019RWM7S6z1UPsZqso4p2HAj
Generated by Claude Code