Fix/joss installability#6
Merged
Merged
Conversation
…contract gap, prep JOSS resubmission Modularity & integration release (v1.4.4) responding to JOSS pre-review feedback (joss-reviews#10182/#10183) on install footprint and the VideoAnnotator/VAV relationship: - FastAPI now serves Video Annotation Viewer's static build at /viewer, same-origin, zero-config (VIDEOANNOTATOR_ENABLE_VIEWER to disable). VAV remains fully independent. - New tests/contract/test_viewer_contract.py models VAV's actual Zod schemas in Python and validates VideoAnnotator's real exporter output against them. First run caught a live gap: person_tracking COCO output was missing person_id/person_label/label_confidence/ labeling_method whenever identity labeling was disabled, which VAV requires as non-optional. Fixed in person_pipeline.py with sensible fallback defaults; also corrected the pipeline's self-declared schema (image_id documented as integer, actually always a string). - Added roadmap_v1.6.0.md (plugin ecosystem + local LLM/Ollama backend) and rewrote roadmap_v1.5.0.md around the modularity spec (specs/003-modular-pipeline-architecture); archived the superseded UX wishlist rather than deleting it. - Doc sweep: README's install flow now leads with the bundled viewer; fixed several stale version pins (INSTALLATION.md still titled "v1.2.0", Docker.md, output_naming_conventions.md). - Added the interface figure to the combined JOSS paper and strengthened the AI-usage disclosure with a concrete, checkable pointer to the specs/roadmap design trail, addressing the "evidence of iterative design process" concern raised during pre-review. - Version bumped to 1.4.4; CITATION.cff was still stale at 1.4.2, also corrected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- paper.md/cover_letter.md: document the /viewer integration and the contract test (with its real caught-and-fixed bug) as concrete evidence of active, coordinated maintenance between VideoAnnotator and Video Annotation Viewer; bump paper date. - Remove paper/release_note_v1.4.2.md: duplicated CHANGELOG.md's existing [1.4.2] entry, wrong location/naming convention, and still had the stale "parallel submission" framing superseded by the combined-paper decision. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add the /viewer bundling and the contract test (with its real caught-and-fixed bug) to paper.md's Summary/Quality control sections and cover_letter.md, as concrete evidence of active, coordinated maintenance between VideoAnnotator and Video Annotation Viewer. Bump paper date to 8 July 2026. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…uickstart Step 3 still buried a full git-clone/npm-install fallback under the bundled /viewer path, which undercut the "no extra install needed" quickstart. Moved the standalone instructions up into the section that already introduces VAV as an independent tool; Step 3 is now just "open /viewer." Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR improves JOSS-facing installability and integration by bundling the Video Annotation Viewer (VAV) static build into the Python package, mounting it at /viewer in the FastAPI app, and adding a contract test to prevent output/schema drift between VideoAnnotator exporters and VAV parsers. It also addresses Windows SQLite teardown issues by adding an explicit storage-backend close() hook and using it in test/database reset paths, and updates release metadata/docs accordingly.
Changes:
- Bundle and serve the Video Annotation Viewer static build at
/viewer(toggle viaVIDEOANNOTATOR_ENABLE_VIEWER), plus CLI/docs updates to advertise it. - Add a cross-repo contract test for COCO/WebVTT/RTTM compatibility and fix person-tracking COCO identity fields to always be present.
- Add storage-backend lifecycle cleanup (
close()) and update reset/teardown logic to reduce Windows file-handle issues; bump version to 1.4.4 and update paper/spec docs.
Reviewed changes
Copilot reviewed 30 out of 42 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/contract/test_viewer_contract.py | New contract tests validating VideoAnnotator exporter output against VAV expectations. |
| tests/conftest.py | Improves Windows teardown by disposing engines / clearing cached backends after tests. |
| src/videoannotator/viewer_static/robots.txt | Adds robots rules to the vendored viewer static directory. |
| src/videoannotator/viewer_static/placeholder.svg | Adds a placeholder SVG asset for the vendored viewer build. |
| src/videoannotator/viewer_static/index.html | Adds the viewer entry HTML referencing bundled assets under /viewer. |
| src/videoannotator/viewer_static/404.html | Adds SPA-style fallback HTML for static hosting under /viewer. |
| src/videoannotator/viewer_static/assets/openface3Parser-Bf49xR9R.js | Adds a bundled/minified viewer parser asset. |
| src/videoannotator/viewer_static/assets/index-BmyNzjfh.css | Adds bundled/minified viewer stylesheet. |
| src/videoannotator/version.py | Bumps package version and release date to 1.4.4 / 2026-07-08. |
| src/videoannotator/storage/sqlite_backend.py | Adds close() to dispose the SQLAlchemy engine (Windows file-handle release). |
| src/videoannotator/storage/base.py | Adds a default no-op close() hook to the storage backend interface. |
| src/videoannotator/pipelines/person_tracking/person_pipeline.py | Ensures COCO identity fields are always present; updates schema types/required fields. |
| src/videoannotator/config_env.py | Adds ENABLE_VIEWER env toggle and prints it in print_config(). |
| src/videoannotator/cli.py | Prints the viewer URL when enabled. |
| src/videoannotator/api/main.py | Mounts the bundled viewer static build at /viewer when enabled and present. |
| src/videoannotator/api/database.py | Ensures cached storage backend is closed before being reset. |
| specs/003-modular-pipeline-architecture/spec.md | Adds a modular pipeline architecture spec document for v1.5+ planning. |
| specs/003-modular-pipeline-architecture/checklists/requirements.md | Adds a spec quality checklist for the modular architecture spec. |
| README.md | Updates docs to describe VAV as bundled and served at /viewer (with standalone option). |
| pyproject.toml | Bumps version to 1.4.4 and includes viewer_static/**/* as package data. |
| paper/paper.md | Updates the JOSS paper to cover VideoAnnotator + VAV as a combined workflow/tooling story. |
| paper/paper.bib | Adds citations used by the updated paper. |
| paper/cover_letter.md | Updates cover letter to reflect the combined submission and integration changes. |
| docs/usage/GETTING_STARTED.md | Documents the bundled /viewer endpoint and how to disable it. |
| docs/README.md | Updates roadmap links and adds pointer to the modular architecture spec. |
| docs/installation/INSTALLATION.md | Updates installation docs (viewer is bundled; Node only for standalone viewer dev). |
| docs/development/roadmap_v1.6.0.md | Adds v1.6.0 roadmap focusing on plugins/local-LLM backends. |
| docs/development/roadmap_v1.5.0.md | Refocuses v1.5.0 roadmap on modularity + viewer integration for JOSS resubmission. |
| docs/development/output_naming_conventions.md | Generalizes naming conventions doc header/status wording. |
| docs/deployment/Docker.md | Notes current images bundle all deps; points to v1.5 plan for slim images. |
| docs/archive/development/roadmap_v1.5.0_ux_wishlist.md | Archives the previous v1.5 UX wishlist plan. |
| CITATION.cff | Updates citation version/date to 1.4.4 / 2026-07-08. |
Files not reviewed (1)
- src/videoannotator/viewer_static/assets/index-BmyNzjfh.css: Generated file
| <link rel="icon" type="image/png" sizes="16x16" href="/viewer/icon-16x16.png" /> | ||
| <link rel="shortcut icon" href="/viewer/favicon.ico" /> | ||
| <link rel="apple-touch-icon" href="/viewer/apple-touch-icon.png" /> | ||
| <script>console.log('🚀 Index.html loaded - browser is processing HTML');</script> |
| <link rel="icon" type="image/png" sizes="16x16" href="/viewer/icon-16x16.png" /> | ||
| <link rel="shortcut icon" href="/viewer/favicon.ico" /> | ||
| <link rel="apple-touch-icon" href="/viewer/apple-touch-icon.png" /> | ||
| <script>console.log('🚀 Index.html loaded - browser is processing HTML');</script> |
| @@ -0,0 +1 @@ | |||
| class s{static instance;static getInstance(){return s.instance||(s.instance=new s),s.instance}parseOpenFace3Data(e){try{if(!this.isValidOpenFace3Data(e))throw new Error("Invalid OpenFace3 data structure");const a=e.faces.map((n,i)=>this.convertFaceAnnotation(n,i));return console.log(`✓ OpenFace3 Parser: Converted ${a.length} face annotations`),a}catch(t){throw console.error("OpenFace3 parsing failed:",t),new Error(`Failed to parse OpenFace3 data: ${t}`)}}convertFaceAnnotation(e,t){return{annotation_id:e.annotation_id||t,bbox:e.bbox,timestamp:e.timestamp,face_confidence:e.features.confidence,openface3:{landmarks_2d:e.features.landmarks_2d,action_units:e.features.action_units,head_pose:e.features.head_pose,gaze:e.features.gaze,emotion:e.features.emotion}}}isValidOpenFace3Data(e){if(!e||typeof e!="object")return!1;const t=e;if(!t.metadata||!t.faces||!Array.isArray(t.faces))return!1;const a=t.metadata;if(!a.pipeline||!a.model_info)return!1;const n=t.faces;if(n.length>0){const i=n[0];if(!i.bbox||!i.features||i.timestamp===void 0)return!1}return!0}getAvailableFeatures(e){const t=e.metadata.model_info.features,a=[];return t.landmarks&&a.push("landmarks_2d"),t.action_units&&a.push("action_units"),t.head_pose&&a.push("head_pose"),t.gaze&&a.push("gaze"),t.emotions&&a.push("emotions"),t.face_tracking&&a.push("face_tracking"),a}getProcessingStats(e){return{totalFaces:e.metadata.processing_stats.total_faces,avgProcessingTime:e.metadata.processing_stats.avg_processing_time,detectionConfidence:e.metadata.config.detection_confidence,modelInfo:`${e.metadata.model_info.model_name} v${e.metadata.model_info.version}`}}filterByConfidence(e,t=.5){return e.filter(a=>(a.face_confidence||0)>=t)}getTimestamps(e){const t=e.map(a=>a.timestamp);return[...new Set(t)].sort((a,n)=>a-n)}}s.getInstance();export{s as OpenFace3Parser}; | |||
Comment on lines
+97
to
+103
| from .config_env import ENABLE_VIEWER | ||
|
|
||
| if ENABLE_VIEWER: | ||
| display_host = "localhost" if host == "0.0.0.0" else host | ||
| typer.echo( | ||
| f"[INFO] Video Annotation Viewer available at http://{display_host}:{port}/viewer" | ||
| ) |
Comment on lines
+165
to
+173
| # Dispose engines before temp dir cleanup — Windows holds files open until | ||
| # all SQLAlchemy connection pools are explicitly released (WinError 32). | ||
| try: | ||
| import videoannotator.database.database as _db_module | ||
|
|
||
| if hasattr(_db_module, "engine"): | ||
| _db_module.engine.dispose() | ||
| except Exception: | ||
| pass |
Comment on lines
175
to
180
| try: | ||
| from videoannotator.api.database import reset_storage_backend | ||
|
|
||
| reset_storage_backend() | ||
| except ImportError: | ||
| reset_storage_backend() # now calls backend.close() before clearing | ||
| except Exception: | ||
| pass |
Comment on lines
182
to
187
| try: | ||
| from videoannotator.storage.manager import get_storage_provider | ||
|
|
||
| get_storage_provider.cache_clear() | ||
| except ImportError: | ||
| except Exception: | ||
| pass |
Comment on lines
+108
to
+117
| value = annotation[field] | ||
| if value is None: | ||
| # Zod .optional() (not .nullable()) rejects an explicit null. | ||
| violations.append(f"field '{field}' is null, which Zod .optional() rejects") | ||
| continue | ||
| if not isinstance(value, expected_types): | ||
| violations.append( | ||
| f"field '{field}' is {type(value).__name__}, expected one of {expected_types}" | ||
| ) | ||
| return violations |
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.
No description provided.