feat(index): add bulk indexing for many media items - #158
Open
Hasnain2430 wants to merge 1 commit into
Open
Conversation
`vidxp index create` handled one media item per invocation, so indexing a repository meant driving it once per video and tracking the results by hand. Add a transport-neutral planning operation and a thin CLI adapter over it: - `Application.plan_bulk_index` resolves a selection to per-media targets and decides which ones the active snapshot already covers. The plan is read-only, so callers can show it before committing to any work. - `vidxp index bulk` indexes every registered media item, or a selection passed with repeated `--media-id`. `--plan-only` shows the decision without indexing, and `--reindex` plans covered media anyway. Media is skipped when the active snapshot holds a generation for it, that generation covers every requested modality, and its recorded input checksum still matches the registered media. Replacing a video's content or asking for a modality the generation lacks therefore plans it again. Media that is not in the ready state is reported as skipped rather than silently dropped. No new indexing behavior. Each pending target is submitted through the existing `submit_index` durable job, one job per media item, matching how `IngestionCoordinator` already sequences ingestion. That is what gives the batch its guarantees: a failure isolates to its own media, earlier successes stay committed, and rerunning the command retries only what is still missing because completed media is then skipped. The command exits non-zero when any media failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Related issue
Closes #68.
Summary
vidxp index createhandles one media item per invocation, so indexing a repository meant driving it once per video and tracking results by hand. This adds a transport-neutral planning operation and a thin CLI adapter over it.Application.plan_bulk_indextakes aPlanBulkIndexCommandand returns aBulkIndexPlanof per-media targets, eachpendingorskippedwith a reason. The plan is read-only, so any interface can show the decision before committing to work, and the HTTP, MCP, and Desktop adapters can reuse it unchanged.vidxp index bulkindexes every registered media item, or a selection passed with repeated--media-id.--plan-onlyshows the decision without indexing,--reindexplans covered media anyway, and--jsonemits the same payload for machine callers.Skip rule. A media item is skipped when the active snapshot holds a generation for it, that generation covers every requested modality, and its recorded
input_sha256still matches the registered media checksum. Replacing a video's content or requesting a modality the generation lacks therefore plans it again. Media that is not in thereadystate is reported asskippedwithmedia_not_readyrather than silently dropped.No new indexing behavior. Each pending target goes through the existing
submit_indexdurable job, one job per media item. That mirrors howIngestionCoordinatoralready sequences ingestion, and it is what gives the batch its guarantees rather than new bookkeeping:vidxp jobs retry <job-id>.The command exits non-zero when any media failed, so it composes in scripts.
User-visible. New CLI command and new application operation; no existing contract changes, and no storage or model change, so existing repositories do not need rebuilding.
Notes for review
bulksubcommand rather than adding--alltoindex create, becausecreatetakes a required positional media identifier and overloading it would change an existing contract. Happy to reshape this if you would rather it be one command.bulk_indexworkflow would have needed entries inJobKind,WORKFLOW_NAMES, theJobRequestandJobResultunions,dbos_workflows.py, anddbos_jobs.py, and it would have duplicated sequencing that per-item jobs already provide.max_concurrent_indexingdefaults to 1, so per-item jobs serialize on their own rather than contending.plan_bulk_indexmirrors the one increate_index. Every capability in the current registry has a collection name, so that branch is not reachable today; the test covers the reachable guard (an unknown capability is rejected) instead of inventing a fake capability to reach it.tests/test_bulk_index.pyis written as aunittest.TestCasedeliberately. CI discovers tests withpython -m unittest discover, which does not collect module-level pytest functions, so a pytest-style file would not have run in CI at all (reported as [Bug]: CI skips 85 tests because unittest discovery cannot collect module-level pytest functions #156).Validation
Windows 11, Python 3.12.14,
scenemodel artifacts prepared. Every command below was run without piping, so the reported exit codes are the commands' own.Unit tests —
uv run --no-sync python -m unittest discover -s tests -p "test_bulk_index.py"→Ran 10 tests ... OK. Covers the skip rule, a generation missing a requested modality, a changed media checksum,--reindex, non-ready media, explicit versus repository-wide selection, cursor pagination across every page, and rejection of an unknown capability.Lint —
uvx "ruff~=0.16.1" check src tests→ all checks passed. Also--select RUF100to confirm no unused suppressions.Documentation —
npx --yes markdownlint-cli2@0.23.2 "INSTALLATION_GUIDE.md"→ exit 0. No links added, so I did not runlycheelocally; please let the documentation workflow cover it.Real media, real model, real index. Two generated 3-second H.264 clips imported into an isolated
--data-dir, indexed with thescenecapability againstgoogle/siglip2-base-patch16-224. This exercised actual FFmpeg decoding, the model provider, the durable job boundary, and committed snapshot storage — not mocks.index bulk --modality scene --plan-only --jsonplanned 2, skipped 0, 2 items listed, nothing indexed, exit 0index bulk --modality scene --jsonindexed 2, skipped 0, failed 0, exit 0index bulk --modality scene --jsonplanned 0, skipped 2bothalready_indexedwith their generation ids, exit 0index bulk --modality scene --reindex --jsonindexed 2, skipped 0, exit 0index bulk --media-id <one> --modality scene --plan-onlyindex status --jsonstate ready, media_count 2, modalities ['scene']search scene "colour bars test pattern" --jsonFailure isolation, verified rather than assumed. In a second isolated repository I imported both clips and deleted the stored content object for one of them, then ran the batch:
index bulk --modality scene --jsonindexed 1, failed 1, exit 1.one.mp4 -> indexed;two.mp4 -> failed: The requested media was not found.skipped 1, planned 1, failed 1, exit 1.one.mp4 -> skipped already_indexed; only the failed item was retried.That is the issue's core guarantee shown end to end: the successful item stayed committed, the failure was reported per media with its reason, and the rerun retried only what was still missing.
I also ran an earlier batch before preparing model artifacts. Both items failed with
Model artifacts for the scene capability are not available locally, the batch continued rather than aborting on the first error, and the command exited 1 — the unhappy path behaves the same way.Full suite —
uv run --no-sync python -m pytest -qon this branch:Both failures are the two Windows path assertions this branch does not contain a fix for, since it branches from
upstream/main. They aretest_codex_plugin.py::test_export_codex_plugin_materializes_the_canonical_skill_bundleandtest_mcp.py::MCPTests::test_clip_submission_and_lazy_artifact_download, both sent separately as #154. Cherry-picking that commit onto this branch and rerunning the two affected files gives60 passed, 7 subtests passed, so this change introduces no failures. I have not merged #154 into this branch, to keep the diff to one outcome.One process note in case it saves someone else the confusion: an earlier full-suite run on this branch reported six failures, including four in
test_native_ingestion.py. That was my own fault — I was running real-media indexing concurrently with the suite, and those end-to-end tests assert that autonomous indexing completes without status polling, which does not hold when the machine is saturated and models are being loaded by another process. Run on their own they pass (4 passed in 98.74s), and the clean full-suite run above is the one to trust.