feat: extras-based modular install & registry refactor (v1.5.0) - #7
Merged
Conversation
Concrete, implementation-ready slice of spec 003 covering the part of v1.5.0 that hasn't been started: moving heavy ML deps to optional-dependencies groups, metadata-driven registry loading (requires_extras), killing LEGACY_MAPPINGS, migration messaging, and the NumPy 2 bump. Adds explicit forward-compatibility requirements (FR-010..FR-012, SC-007) so this phase's schema choices don't have to be redone when v1.6.0 adds Ollama or v1.7+ adds remote/HPC dispatch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…updates from v1.5.0 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Phase 0 (research.md) traced actual per-pipeline imports rather than trusting the informal requirements.packages YAML field, and found the extras taxonomy needs adjusting from spec/roadmap assumptions: no pipeline declares pipeline_family: embedding (openface3 embedding is a face variant), and scene/person both pull torch transitively via open-clip/ultralytics, so they're not as slim as face's DeepFace variant. Settled on face/face-laion/face-openface3/audio/audio-laion/ scene/person/all as the extras groups, with the llm group name reserved but not created (v1.6.0). Phase 1 (data-model.md, contracts/, quickstart.md) specifies the additive requires_extras field on PipelineMetadata, the pipeline-metadata YAML schema delta, and the unavailable-pipeline error contract (CLI/API), each cross-checked against the ratified constitution's Plan Gating checklist (Constitution Check: no violations). Also creates CLAUDE.md (the repo had none) via update-agent-context.sh, then hand-fixed the auto-generated content — the simple field parser truncated the Technical Context strings and left template placeholders unresolved. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
38 tasks across Setup (1), Foundational (13), US1/MVP (9), US2 (8), US3 (3), and Polish (4). Verified against the actual codebase rather than the spec's abstractions alone: all 9 pipeline metadata YAML files currently rely entirely on LEGACY_MAPPINGS (none has module_path or requires_extras today), so Foundational includes one task per file rather than a generic "update metadata" task. Fixed an off-by-one in plan.md's Scale/Scope (said 8 metadata files, it's 9). Also corrects the fact that this dependency graph surfaces: US2's migration-message task (T027) is the only real cross-story coupling, sharing US1's error-path code (T022) — everything else in US1/US2/US3 is independently staffable after the Foundational phase. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Implements specs/004-extras-based-install: moves heavy ML pipeline dependencies out of the core install into opt-in pip extras (face, face-laion, face-openface3, audio, audio-laion, scene, person), and replaces the registry's hardcoded pipeline<->module mapping with metadata-driven resolution (PipelineMetadata.requires_extras, required module_path, LEGACY_MAPPINGS removed). Bugs found and fixed via real pip-install + running-server testing, not just unit tests against the dev venv: - face extras missing tf-keras (deepface/retinaface import failure) - registry/metadata/*.yaml never declared in package-data, so a real install resolved zero pipelines regardless of extras (masked pre-refactor by the LEGACY_MAPPINGS fallback this phase removes) - videoannotator/pipelines/__init__.py (and audio_processing/, face_analysis/ one level down) eagerly imported every pipeline family at package-init time, so loading any single pipeline forced every other family's heavy deps to import too -- no pipeline could load except under an [all] install. Fixed with PEP 562 lazy attribute resolution. - face-laion missing a real dependency on face's deepface stack (LAIONFacePipeline composes FaceAnalysisPipeline as its detector) - opencv-python/opencv-python-headless coexistence corrupting the shared cv2 install (core no longer declares headless; standardized on plain opencv-python everywhere), plus opencv-python 5.0 dropping cv2.CascadeClassifier entirely (pinned <5.0) - open-clip-torch 3.1.0 API drift breaking scene classification (model() now returns image/text features + logit_scale, not logits_per_image/text directly) - torch/tensorflow cuDNN version conflict (torch hard-pins 9.1.0.70, tensorflow wants >=9.3.0.75) causing deepface GPU failures; forced TensorFlow onto CPU via its own device-visibility API, torch's GPU usage unaffected (VIDEOANNOTATOR_DEEPFACE_GPU=1 to opt out) - registry.get_registry() wasn't exported from videoannotator.registry (only the submodule), and __init__.py carried a second, unused, orphaned PipelineRegistry singleton -- removed it Also drops the numpy<2.0 pin (numpy now resolves per numba's own declared ceiling) and retires the now-redundant numpy2-test CI job. Bumps package version to 1.5.0. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Checks off criteria confirmed this session (no-extras install has no heavy deps, actionable install-hint errors, Linux NumPy 2.x suite) and marks two as partially verified (install-to-viewer flow modulo an actual browser check; the viewer contract test's Python-side, which already runs in CI, vs. its still-deferred TypeScript-side). Leaves three genuinely open with explicit reasons rather than silently unchecked: Docker image size (no docker binary in this sandbox), macOS/Windows suite runs (Linux-only sandbox, needs real CI), and true byte-identical v1.4.3 parity (needs golden fixtures captured from the v1.4.4 tag, not done in this pass). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR delivers the v1.5.0 “extras-based modular install” refactor: heavy ML dependencies are moved out of the core install into opt-in extras groups, and pipeline discovery/loading is shifted from hardcoded mappings to metadata-driven module_path + requires_extras gating, with CLI/API surfaces updated to communicate “unavailable pipeline” situations via actionable install hints.
Changes:
- Refactors the registry/loader to be fully metadata-driven (
module_pathrequired) and extras-aware, including migration notes for pipelines demoted from the v1.4.4 default install. - Updates API + CLI to omit unavailable pipelines by default (with an include-unavailable mode) and to return/print consistent install-hint errors for unavailable pipelines.
- Updates packaging (extras definitions, shipped registry metadata), Dockerfiles, CI, docs/spec artifacts, and adds tests covering the new contracts.
Reviewed changes
Copilot reviewed 71 out of 72 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/registry/test_pipeline_registry.py | Unit coverage for requires_extras, required module_path, and extras helper functions. |
| tests/unit/registry/test_forward_compat_stub.py | Forward-compat regression test proving empty requires_extras works with a stub pipeline. |
| tests/integration/test_v144_parity.py | Scaffolded parity test structure vs v1.4.4 golden fixtures (skips until fixtures exist). |
| tests/integration/test_model_cache_reuse.py | Ensures registry/loader paths do not mutate HF/Torch cache directories. |
| tests/integration/test_extras_isolation.py | Opt-in test that installs extras in a clean venv and validates dependency isolation. |
| tests/fixtures/stub_pipeline.yaml | Fixture metadata file for the forward-compat stub pipeline. |
| tests/fixtures/stub_pipeline_module.py | Stub BasePipeline implementation used by the forward-compat test. |
| tests/contract/test_unavailable_pipeline_error.py | Contract coverage for API/CLI unavailable-pipeline error shape and messaging. |
| tests/contract/test_pipeline_availability_contract.py | Contract coverage for pipeline listing omission/include behavior + install hints. |
| tests/contract/test_migration_message_contract.py | Contract coverage for “demoted from default install” migration messaging. |
| src/videoannotator/version.py | Bumps package version metadata to 1.5.0. |
| src/videoannotator/utils/audio.py | Makes librosa import lazy to keep core installs working without audio extras. |
| src/videoannotator/registry/pipeline_registry.py | Adds requires_extras + module_path; enforces module_path as required at load time. |
| src/videoannotator/registry/pipeline_loader.py | Implements extras availability checks from installed metadata; removes legacy mappings; adds migration note + install hint helpers. |
| src/videoannotator/registry/metadata/speech_recognition.yaml | Adds module_path + requires_extras for metadata-driven loading. |
| src/videoannotator/registry/metadata/speaker_diarization.yaml | Adds module_path + requires_extras for metadata-driven loading. |
| src/videoannotator/registry/metadata/scene_detection.yaml | Adds module_path + requires_extras for metadata-driven loading. |
| src/videoannotator/registry/metadata/person_tracking.yaml | Adds module_path + requires_extras for metadata-driven loading. |
| src/videoannotator/registry/metadata/laion_voice.yaml | Adds module_path + requires_extras for metadata-driven loading. |
| src/videoannotator/registry/metadata/face_openface3_embedding.yaml | Adds module_path + requires_extras for metadata-driven loading. |
| src/videoannotator/registry/metadata/face_laion_clip.yaml | Adds module_path + requires_extras for metadata-driven loading. |
| src/videoannotator/registry/metadata/face_analysis.yaml | Adds module_path + requires_extras for metadata-driven loading. |
| src/videoannotator/registry/metadata/audio_processing.yaml | Adds module_path + requires_extras for metadata-driven loading. |
| src/videoannotator/registry/init.py | Removes global pipeline_registry instance export; exports get_registry. |
| src/videoannotator/pipelines/scene_detection/scene_pipeline.py | Adapts scene classification to open_clip API changes by computing logits from returned features. |
| src/videoannotator/pipelines/face_analysis/face_pipeline.py | Adds TensorFlow GPU-disable workaround to avoid cuDNN pin conflicts in DeepFace GPU environments. |
| src/videoannotator/pipelines/face_analysis/init.py | Converts face pipeline exports to PEP-562 lazy attribute loading to preserve extras isolation. |
| src/videoannotator/pipelines/audio_processing/init.py | Lazily loads LAIONVoicePipeline to avoid importing audio-laion deps on [audio] installs. |
| src/videoannotator/pipelines/init.py | Converts pipeline family exports to PEP-562 lazy attribute loading to prevent cross-extras eager imports. |
| src/videoannotator/cli.py | Improves job-submit errors for 404/422 and adds videoannotator pipelines --all support for unavailable pipelines. |
| src/videoannotator/api/v1/pipelines.py | Adds include_unavailable query param; includes available + install_hint fields. |
| src/videoannotator/api/v1/jobs.py | Detects “recognized but unavailable” pipelines early and returns 422 with install hints. |
| src/videoannotator/api/v1/handlers.py | Promotes structured error detail keys to top-level response fields for contract consumers. |
| src/videoannotator/api/v1/exceptions.py | Adds PipelineUnavailableException with migration note + install hint payload. |
| specs/004-extras-based-install/research.md | Captures research decisions and rationale behind extras grouping + loader approach. |
| specs/004-extras-based-install/quickstart.md | Provides manual verification steps aligned to acceptance scenarios for extras installs. |
| specs/004-extras-based-install/plan.md | Implementation plan artifact for the extras-based install/refactor feature. |
| specs/004-extras-based-install/data-model.md | Defines the extended metadata model and semantics (requires_extras, required module_path). |
| specs/004-extras-based-install/contracts/unavailable-pipeline-error.md | Contract for unavailable-pipeline behavior across API/CLI/listing surfaces. |
| specs/004-extras-based-install/contracts/pipeline-metadata-schema.md | Contract for YAML schema delta requiring module_path and requires_extras. |
| specs/004-extras-based-install/checklists/requirements.md | Spec quality checklist artifact. |
| pyproject.toml | Splits ML deps into extras; updates all meta-extra; ships registry metadata as package-data; bumps version. |
| docs/installation/INSTALLATION.md | Updates install guidance with extras matrix, upgrade notes, and CLI flags for listing unavailable pipelines. |
| docs/development/roadmap_v1.7_to_v2.0.md | Adds/updates roadmap doc for v1.7→v2.0 arc building on extras + registry work. |
| docs/development/roadmap_v1.5.0.md | Marks v1.5.0 extras-based install work as completed and updates success-criteria status. |
| Dockerfile.gpu | Adds extras build arg flow; defaults to slim image; documents usage. |
| Dockerfile.dev | Installs --extra all explicitly since pipeline deps are no longer core. |
| Dockerfile.cpu | Adds extras build arg flow; installs CPU torch wheels only when extras requested; documents usage. |
| CLAUDE.md | Adds repo guidance snapshot for tooling/structure (agent context). |
| CITATION.cff | Updates version/date-released to 1.5.0. |
| CHANGELOG.md | Adds v1.5.0 release notes and records key fixes/behavior changes. |
| .specify/scripts/bash/setup-tasks.sh | Improves JSON output robustness and template resolution behavior. |
| .specify/scripts/bash/setup-plan.sh | Improves JSON output robustness and branch/feature-dir validation behavior. |
| .specify/scripts/bash/check-prerequisites.sh | Improves JSON output robustness for prereq checks and paths-only mode. |
| .specify/memory/constitution.md | Replaces placeholder constitution with a ratified v1.0.0 constitution document. |
| .specify/feature.json | Pins current feature directory for speckit workflow. |
| .github/workflows/ci-cd.yml | Updates CI install steps to use uv sync --dev --all-extras. |
| .devcontainer/devcontainer.json | Adds GitHub CLI devcontainer feature. |
| .devcontainer/devcontainer-lock.json | Locks devcontainer feature resolution for GitHub CLI. |
| .claude/skills/speckit-tasks/SKILL.md | Adds/updates skill definition file in-repo. |
| .claude/skills/speckit-specify/SKILL.md | Adds/updates skill definition file in-repo. |
| .claude/skills/speckit-plan/SKILL.md | Adds/updates skill definition file in-repo. |
| .claude/skills/speckit-implement/SKILL.md | Adds/updates skill definition file in-repo. |
| .claude/skills/speckit-constitution/SKILL.md | Adds/updates skill definition file in-repo. |
| .claude/skills/speckit-clarify/SKILL.md | Adds/updates skill definition file in-repo. |
| .claude/skills/speckit-analyze/SKILL.md | Adds/updates skill definition file in-repo. |
test (macos-latest) started hard-crashing (Fatal Python error: Aborted, exit 134) during test collection after dropping the numpy<2.0 pin -- numpy 2.x + pyarrow (via datasets/pandas) each bundle their own OpenMP runtime, which can double-initialize alongside TensorFlow's and abort the process on import rather than any actual test failure. Confirmed via CI: master's last green run (numpy 1.26.4) passed macOS cleanly; this branch's only dependency diff is numpy 1.26.4 -> 2.2.6, and pyarrow/pandas versions are otherwise identical. Sets KMP_DUPLICATE_LIB_OK=TRUE for the pytest step, the standard mitigation for this class of crash and the same one already documented for audio pipelines in docs/installation/INSTALLATION.md's macOS libomp note. No-op on Linux/Windows. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
KMP_DUPLICATE_LIB_OK=TRUE (previous commit) did not fix the macOS ARM64 test job's Fatal Python error: Aborted crash -- confirmed via CI, identical crash at pyarrow/__init__.py:61 with the env var set, ruling out an OpenMP double-init conflict. This looks like an ABI issue in pyarrow 21.0.0's compiled extension against numpy 2.2.6 on that specific platform (datasets>=2.16.0 and pandas both leave pyarrow's upper bound unconstrained, so 21.0.0 was just whatever uv's numpy-only upgrade left in place, not a deliberate pin). Forced a fresh resolve of pyarrow specifically (`uv lock --upgrade-package pyarrow`); verified locally on Linux (pyarrow imports cleanly, tests/unit: 646 passed, 0 failed) before pushing to let CI confirm on the actual affected platform. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.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.
Summary
pipextras (face,face-laion,face-openface3,audio,audio-laion,scene,person), replacing the registry's hardcoded pipeline↔module mapping with fully metadata-driven loading (PipelineMetadata.requires_extras, requiredmodule_path,LEGACY_MAPPINGSremoved).pip install+ running-server testing (not just unit tests against the dev venv): missingtf-kerasforface,registry/metadata/*.yamlnever shipped in a real install (zero pipelines loadable), eager cross-extras imports inpipelines/__init__.pydefeating isolation entirely,face-laionmissing its real dependency onface's deepface stack,opencv-python/opencv-python-headlesscoexistence corruption plusopencv-python5.0 droppingcv2.CascadeClassifier, anopen-clip-torch3.1.0 API drift breaking scene classification, a torch/tensorflow cuDNN version conflict breaking DeepFace on GPU, and an unexported/duplicated registry singleton.numpy<2.0pin (resolves pernumba's own declared ceiling) and retires the now-redundantnumpy2-testCI job.docs/development/roadmap_v1.5.0.md's success criteria with honest, verified-or-explicitly-open status per item.Full details in
CHANGELOG.mdandspecs/004-extras-based-install/tasks.md.Test plan
pytest(unit + contract + pipelines + integration): 1066 passed, 32 skipped, 0 failedruff check/ruff format --check/mypy src/videoannotator: cleanspecs/004-extras-based-install/quickstart.md) run against realpip install+ running server, not just the dev venvdockerbinary in this session's sandbox)🤖 Generated with Claude Code