Add video asset support with dual-pass search#62
Merged
Merged
Conversation
Implement assetTypesForSearch() to conditionally enumerate IMAGE and VIDEO types. Refactor FetchAssets, FetchTrashedAssets, and fetchAllAssetIDsViaSearch to paginate across all requested types with deduplication. Since Immich's /search/metadata endpoint accepts only one type per request, querying twice with a seen map prevents duplicate results while enabling callers to search videos. This unlocks stacking for video files, Live Photo pairs (HEIC+MOV), and edited video variants without breaking existing image-only workflows. Change-Type: feature Scope: immich
Add includeVideos boolean flag to control video asset enumeration. Support both CLI flag (--include-videos) and environment variable (INCLUDE_VIDEOS=true) for configuration. Wire into startup logging so users can verify the setting is active. Change-Type: feature Scope: config
Update NewClient calls in stacker, duplicates, and fixtrash commands to pass the includeVideos flag. Ensures consistent video asset behavior across all CLI entry points. Change-Type: refactor Scope: cmd
Update existing client initialization tests with new includeVideos parameter. Create dedicated test suite for video functionality: verify assetTypesForSearch() returns correct types, confirm FetchAssets/FetchTrashedAssets dispatch appropriate /search/metadata calls per asset type, validate hybrid fallback (fetchAllAssetIDsViaSearch) respects includeVideos across visibility passes. Change-Type: test Scope: immich
Add new troubleshooting section explaining video file stacking use cases (Live Photos, trimmed videos, burst videos) and performance implications. Document INCLUDE_VIDEOS flag, explain the dual-pass pagination mechanism, and reference issue #54. Change-Type: docs Scope: troubleshooting
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds opt-in video asset support to the Immich client and CLI by allowing /search/metadata enumeration to run for both IMAGE and VIDEO types and deduplicating results, enabling stacking behavior to consider videos when configured.
Changes:
- Added
includeVideosplumbing from CLI/env config into the Immich client constructor. - Implemented dual-pass
/search/metadataenumeration (IMAGE then VIDEO) with deduplication in asset-fetching paths. - Added video-focused unit tests and updated troubleshooting docs with configuration guidance.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/immich/client.go | Adds includeVideos config, introduces assetTypesForSearch(), and updates asset/trash fetching to enumerate IMAGE/VIDEO with dedupe. |
| pkg/immich/client_hybrid_helpers.go | Extends hybrid /search/metadata ID enumeration to run per asset type when videos are enabled. |
| pkg/immich/client_test.go | Updates NewClient test call sites for the new includeVideos parameter. |
| pkg/immich/client_videos_test.go | Adds tests validating that includeVideos results in /search/metadata calls for both IMAGE and VIDEO. |
| cmd/main.go | Adds --include-videos persistent flag. |
| cmd/config.go | Introduces global includeVideos, logs it in startup summary, and loads INCLUDE_VIDEOS from env. |
| cmd/stacker.go | Passes includeVideos into immich.NewClient for stacker flows. |
| cmd/duplicates.go | Passes includeVideos into immich.NewClient for duplicates flow. |
| cmd/fixtrash.go | Passes includeVideos into immich.NewClient for fix-trash flow. |
| docs/troubleshooting.md | Documents how to enable video stacking and the performance implications. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Three findings from Copilot on PR #62: - NewClient doc comment now lists the new includeVideos parameter alongside the others. - Flag-vs-env precedence for include-videos now follows the same pattern as replace-stacks: a separate includeVideosFlagSet sentinel marks when the CLI flag was explicitly provided, so --include-videos=false correctly overrides INCLUDE_VIDEOS=true (the documented precedence rule). - typeTrackingTransport in the test suite no longer holds its mutex during request-body I/O and JSON decoding, and closes req.Body after reading.
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
Introduces comprehensive video asset support to immich-stack, allowing users to include videos in the stacking logic. The feature includes a new --include-videos flag, environment variable configuration, and a dual-pass search mechanism to efficiently handle video assets alongside photos.
What changed
Risks
Test plan
go test ./...to ensure no regressionsgo test -run TestVideo ./pkg/immich -vDocs impact
Updated docs/troubleshooting.md with video stacking configuration guidance, including examples of --include-videos flag usage and INCLUDE_VIDEOS environment variable setup.
Breaking changes
None - the feature is fully opt-in via the --include-videos flag and INCLUDE_VIDEOS environment variable. Existing configurations and workflows are unaffected.