Read the JSON sidecar for videos generated before InvokeAI 7 - #401
Merged
Merged
Conversation
…AI 7
Pre-7 releases kept a generated video's record in a sidecar under
{outputs}/videos/sidecars/, mirroring the video's own subfolder, rather than
inside the MP4. InvokeAI 7 still writes one when the embedding remux fails,
so this is a live fallback and not only a legacy path.
The extractor now reads the MP4's keyed metadata first and the sidecar
second, which is InvokeAI's own order and means a v7 video costs no sidecar
lookup at all.
Finding the sidecar is the interesting part: PhotoMapAI indexes absolute
paths and never learns where an `outputs` directory begins, so each ancestor
of the video is tried as the videos root, nearest first, mirroring the
video's relative path under `sidecars/`. Bounded, and required to carry an
`invokeai_metadata` key, so an unrelated directory called `sidecars` cannot
be mistaken for InvokeAI's.
Measured against a real install (668 videos, 662 sidecars): every sidecar
resolved at ancestor depth 1, 132 carried a record and 530 carried a null
one (a workflow but no parameters), 3 videos had the record embedded, and
the whole scan cost 1 ms per video including the MP4 walk. All 132 recovered
records parse, are recognised as video generations, and render a panel with
no undeclared fields — which also validates the v5 video profile against
real data rather than only against the synthetic fixture.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… reader Six defects, each reproduced first, and the fixes mutation-tested after. - RecursionError escaped the reader. json.loads raises it on deeply nested JSON; it is a RuntimeError, so neither `except OSError` nor `except ValueError` caught it, and ~120 KB of brackets is three orders of magnitude under the size cap. The damage was not a lost record but a lost *video*: _load_video catches it, returns None, and the file is recorded as bad and left out of the album — exactly what the docstring promises cannot happen. Both json.loads sites were exposed; they now share a helper. - The `invokeai_metadata` key gate was weak and untested: it handed back whatever object another tool stored under that name, and removing the gate entirely left all 25 tests passing. The record must now also satisfy looks_like_invoke_metadata — the same test the drawer routes on, and one all 132 records in the reference install pass. - MAX_SUBFOLDER_DEPTH cut from 3 to 1. The bound had no real test (the old one imported the constant it was checking, so it passed at 1 and at 9) and no evidence: 661 of 661 real sidecars resolve at depth 1. The extra levels only bought reach *outside* the configured album — at depth 3, a path at the filesystem root. That also removes the `..` escape the review found, which needed depth 2. - The reader resolves the video path first, so a symlinked video finds the sidecar beside its target. sidecar_candidates stays pure and now documents that it expects a resolved path. - Size cap 32 MiB -> 8 MiB, matching mp4_metadata.MAX_TAG_BYTES, so the same record is not rejected from one source and accepted from the other. - UnicodeDecodeError was caught (it is a ValueError) but logged as a JSON error; split so the message names the right stage. Also pinned the empty-embedded-record fall-through, which the `or` cannot distinguish from "no tag", in both a test and the docstring. Re-measured against the reference install after the fixes: the same 135 records recovered from 668 videos, at 0.2 ms per video. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The note shipped in #400 said an update "only re-reads files whose modification time has changed", which implies Update Index might pick up newly-readable metadata. It never will: _get_new_and_missing_images is a set difference on paths, and modification_times is stored and sorted but never consulted to decide re-processing, so a file already in the index is not re-read whatever its mtime. Confirmed on a real album: 363 indexed videos, 4 carrying a record, 89 more recovered only after rebuilding. Points at the Rebuild Index button from #402 rather than telling people to delete the index file by hand, and links to that button's own section, which covers what a rebuild costs and what it leaves alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lstein
force-pushed
the
lstein/feature/video-metadata-sidecars
branch
from
September 20, 2026 18:36
8f9b0ce to
598e3e5
Compare
lstein
enabled auto-merge (squash)
September 20, 2026 18:37
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.
Follow-up to #400. That PR read the generation record InvokeAI 7 embeds inside an MP4. Releases before 7 kept it in a JSON sidecar under
outputs/videos/sidecars/instead, mirroring the video's own subfolder — and InvokeAI 7 still writes one when its embedding remux fails, so this is a live fallback and not only a legacy path.extract_video_metadatanow reads the MP4's keyed metadata first and the sidecar second, which is InvokeAI's own order and means a v7 video costs no sidecar lookup at all.Finding the sidecar
PhotoMapAI indexes absolute paths and never learns where an
outputsdirectory begins, so the videos root has to be guessed: each ancestor of the video is tried as the root, nearest first, mirroring the video's relative path undersidecars/. A candidate must be JSON, hold aninvokeai_metadatakey, and contain something that passeslooks_like_invoke_metadata— the same test the drawer routes on — so asidecarsdirectory belonging to another tool cannot be mistaken for InvokeAI's.The walk stops at one ancestor. That is measured, not guessed: 661 of 661 sidecars in a real install resolve at depth 1 (the
general/intermediatecategory), and depth 0 covers a videos root with no subfolder. Going deeper bought nothing and reached outside the album the user configured — at depth 3, a path at the filesystem root.Measured against a real install
668 videos, 662 sidecars, rather than only against the synthetic fixture:
The review pass
The second commit is six defects an adversarial fresh-context review found, each reproduced before fixing. Two are worth a reviewer's attention:
RecursionErrorescaped the reader and deleted videos from the index.json.loadsraises it on deeply nested JSON; it is aRuntimeError, so neitherexcept OSErrornorexcept ValueErrorcaught it, and ~120 KB of brackets is three orders of magnitude under the size cap. The damage was not a lost record:_load_videocatches it, returnsNone, and the file is recorded as bad and left out of the album — exactly what the extractor's docstring promises cannot happen.The key gate was theatre. Removing it entirely left all 25 tests passing, and it accepted
{"invokeai_metadata": {"anything": "at all"}}as a generation record. Hence thelooks_like_invoke_metadatarequirement above, and a test that actually fails without it.The rest: the depth cut above (its old test imported the constant it was checking, so it passed at 1 and at 9), symlink resolution, a size cap now matching
mp4_metadata.MAX_TAG_BYTES, and a log line naming the wrong stage.I mutation-tested the fixes rather than trusting green tests, which caught one of my own: reverting
.resolve()left everything passing, because cutting the depth had already removed the..case the test claimed to pin. It is now a symlink test that genuinely fails without the fix. Reverting each of the other five guards fails a test.Docs
The third commit corrects guidance shipped in #400, which said an update "only re-reads files whose modification time has changed" and so implied Update Index might pick this up. It never will —
_get_new_and_missing_imagesis a set difference on paths, andmodification_timesis never consulted to decide re-processing. An existing album needs a full rebuild, verified on a real album: 363 indexed videos, 4 carrying a record, 89 recovered only after rebuilding. Now that #402 has landed, the note points at its Rebuild Index button rather than telling people to delete the index file by hand.Testing
1126 backend + 892 frontend tests pass; ruff, eslint and prettier clean. Verified in the running app against a real pre-7 collection.
🤖 Generated with Claude Code