From 276949d6d51e85ac6441c439755cf1bf773b856e Mon Sep 17 00:00:00 2001 From: Major Date: Tue, 26 May 2026 19:40:05 +0200 Subject: [PATCH 1/6] add VIDEO asset type support with dual-pass search 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 --- pkg/immich/client.go | 187 ++++++++++++++++------------ pkg/immich/client_hybrid_helpers.go | 66 +++++----- 2 files changed, 140 insertions(+), 113 deletions(-) diff --git a/pkg/immich/client.go b/pkg/immich/client.go index c506c06..314e7ae 100644 --- a/pkg/immich/client.go +++ b/pkg/immich/client.go @@ -71,6 +71,7 @@ type Client struct { withArchived bool withDeleted bool removeSingleAssetStacks bool + includeVideos bool filterAlbumIDs []string filterTakenAfter string filterTakenBefore string @@ -95,7 +96,7 @@ type Client struct { ** @param logger - Logger instance for output ** @return *Client - Configured Immich client instance **************************************************************************************************/ -func NewClient(apiURL, apiKey string, resetStacks bool, replaceStacks bool, dryRun bool, withArchived bool, withDeleted bool, removeSingleAssetStacks bool, filterAlbumIDs []string, filterTakenAfter string, filterTakenBefore string, logger *logrus.Logger) *Client { +func NewClient(apiURL, apiKey string, resetStacks bool, replaceStacks bool, dryRun bool, withArchived bool, withDeleted bool, removeSingleAssetStacks bool, includeVideos bool, filterAlbumIDs []string, filterTakenAfter string, filterTakenBefore string, logger *logrus.Logger) *Client { if apiKey == "" { return nil } @@ -134,6 +135,7 @@ func NewClient(apiURL, apiKey string, resetStacks bool, replaceStacks bool, dryR withArchived: withArchived, withDeleted: withDeleted, removeSingleAssetStacks: removeSingleAssetStacks, + includeVideos: includeVideos, filterAlbumIDs: filterAlbumIDs, filterTakenAfter: filterTakenAfter, filterTakenBefore: filterTakenBefore, @@ -297,6 +299,22 @@ func (c *Client) FetchAllStacks() (map[string]utils.TStack, error) { return stacksMap, nil } +/************************************************************************************************** +** assetTypesForSearch returns the asset types to enumerate when calling /search/metadata. +** Defaults to IMAGE only (historical behavior). When includeVideos is enabled, returns +** [IMAGE, VIDEO] so the caller can run the same pagination twice and deduplicate, since the +** Immich search endpoint only accepts a single type per request. +** +** Returned values must always be non-empty Immich AssetTypeEnum values (IMAGE, VIDEO, AUDIO, +** or OTHER). An empty string would make the server return ALL types — never return one. +**************************************************************************************************/ +func (c *Client) assetTypesForSearch() []string { + if c.includeVideos { + return []string{"IMAGE", "VIDEO"} + } + return []string{"IMAGE"} +} + /************************************************************************************************** ** FetchAssets retrieves all assets from Immich with pagination support. ** Assets are enriched with their stack information if available. @@ -349,63 +367,65 @@ func (c *Client) FetchAssets(size int, stacksMap map[string]utils.TStack) ([]uti seen := make(map[string]bool) var allAssets []utils.TAsset - for _, albumFilter := range albumFilters { - page := 1 - for { - if len(albumFilter) > 0 { - c.logger.Debugf("Fetching page %d for album(s) %v", page, albumFilter) - } else { - c.logger.Debugf("Fetching page %d", page) - } - var response utils.TSearchResponse + for _, assetType := range c.assetTypesForSearch() { + for _, albumFilter := range albumFilters { + page := 1 + for { + if len(albumFilter) > 0 { + c.logger.Debugf("Fetching page %d (%s) for album(s) %v", page, assetType, albumFilter) + } else { + c.logger.Debugf("Fetching page %d (%s)", page, assetType) + } + var response utils.TSearchResponse + + payload := map[string]interface{}{ + "size": size, + "page": page, + "order": "asc", + "type": assetType, + "isVisible": true, + "withStacked": true, + "withArchived": c.withArchived, + "withDeleted": c.withDeleted, + } + if len(albumFilter) > 0 { + payload["albumIds"] = albumFilter + } + if c.filterTakenAfter != "" { + payload["takenAfter"] = c.filterTakenAfter + } + if c.filterTakenBefore != "" { + payload["takenBefore"] = c.filterTakenBefore + } - payload := map[string]interface{}{ - "size": size, - "page": page, - "order": "asc", - "type": "IMAGE", - "isVisible": true, - "withStacked": true, - "withArchived": c.withArchived, - "withDeleted": c.withDeleted, - } - if len(albumFilter) > 0 { - payload["albumIds"] = albumFilter - } - if c.filterTakenAfter != "" { - payload["takenAfter"] = c.filterTakenAfter - } - if c.filterTakenBefore != "" { - payload["takenBefore"] = c.filterTakenBefore - } + if err := c.doRequest(http.MethodPost, "/search/metadata", payload, &response); err != nil { + c.logger.Errorf("Error fetching assets: %v", err) + return nil, fmt.Errorf("error fetching assets: %w", err) + } - if err := c.doRequest(http.MethodPost, "/search/metadata", payload, &response); err != nil { - c.logger.Errorf("Error fetching assets: %v", err) - return nil, fmt.Errorf("error fetching assets: %w", err) - } + // Enrich assets with stack information and deduplicate + for i := range response.Assets.Items { + asset := &response.Assets.Items[i] + if seen[asset.ID] { + continue + } + seen[asset.ID] = true + if stack, ok := stacksMap[asset.ID]; ok { + asset.Stack = &stack + } + allAssets = append(allAssets, *asset) + } - // Enrich assets with stack information and deduplicate - for i := range response.Assets.Items { - asset := &response.Assets.Items[i] - if seen[asset.ID] { - continue + // Handle string nextPage: empty string means no more pages + if response.Assets.NextPage == "" || response.Assets.NextPage == "0" { + break } - seen[asset.ID] = true - if stack, ok := stacksMap[asset.ID]; ok { - asset.Stack = &stack + nextPageInt, err := strconv.Atoi(response.Assets.NextPage) + if err != nil || nextPageInt == 0 { + break } - allAssets = append(allAssets, *asset) - } - - // Handle string nextPage: empty string means no more pages - if response.Assets.NextPage == "" || response.Assets.NextPage == "0" { - break - } - nextPageInt, err := strconv.Atoi(response.Assets.NextPage) - if err != nil || nextPageInt == 0 { - break + page = nextPageInt } - page = nextPageInt } } @@ -526,42 +546,47 @@ func (c *Client) GetCurrentUser() (utils.TUserResponse, error) { **************************************************************************************************/ func (c *Client) FetchTrashedAssets(size int) ([]utils.TAsset, error) { var allTrashedAssets []utils.TAsset - page := 1 + seen := make(map[string]bool) c.logger.Debugf("🗑️ Fetching trashed assets:") - for { - c.logger.Debugf("Fetching trashed assets page %d", page) - var response utils.TSearchResponse - if err := c.doRequest(http.MethodPost, "/search/metadata", map[string]interface{}{ - "size": size, - "page": page, - "order": "asc", - "type": "IMAGE", - "isVisible": true, - "withStacked": true, - "withArchived": false, - "withDeleted": true, - }, &response); err != nil { - c.logger.Errorf("Error fetching trashed assets: %v", err) - return nil, fmt.Errorf("error fetching trashed assets: %w", err) - } + for _, assetType := range c.assetTypesForSearch() { + page := 1 + for { + c.logger.Debugf("Fetching trashed assets page %d (%s)", page, assetType) + var response utils.TSearchResponse + if err := c.doRequest(http.MethodPost, "/search/metadata", map[string]interface{}{ + "size": size, + "page": page, + "order": "asc", + "type": assetType, + "isVisible": true, + "withStacked": true, + "withArchived": false, + "withDeleted": true, + }, &response); err != nil { + c.logger.Errorf("Error fetching trashed assets: %v", err) + return nil, fmt.Errorf("error fetching trashed assets: %w", err) + } - // Filter for only trashed assets - for _, asset := range response.Assets.Items { - if asset.IsTrashed { + // Filter for only trashed assets, deduplicating across type passes + for _, asset := range response.Assets.Items { + if !asset.IsTrashed || seen[asset.ID] { + continue + } + seen[asset.ID] = true allTrashedAssets = append(allTrashedAssets, asset) } - } - // Handle string nextPage: empty string means no more pages - if response.Assets.NextPage == "" || response.Assets.NextPage == "0" { - break - } - nextPageInt, err := strconv.Atoi(response.Assets.NextPage) - if err != nil || nextPageInt == 0 { - break + // Handle string nextPage: empty string means no more pages + if response.Assets.NextPage == "" || response.Assets.NextPage == "0" { + break + } + nextPageInt, err := strconv.Atoi(response.Assets.NextPage) + if err != nil || nextPageInt == 0 { + break + } + page = nextPageInt } - page = nextPageInt } c.logger.Debugf("🗑️ %d trashed assets found", len(allTrashedAssets)) diff --git a/pkg/immich/client_hybrid_helpers.go b/pkg/immich/client_hybrid_helpers.go index f9ab9ed..173aea4 100644 --- a/pkg/immich/client_hybrid_helpers.go +++ b/pkg/immich/client_hybrid_helpers.go @@ -82,40 +82,42 @@ func (c *Client) fetchAllAssetIDsViaSearch() ([]string, error) { var ids []string visibilityFilters := []interface{}{nil, "archive"} - for _, vis := range visibilityFilters { - page := 1 - const pageSize = 1000 - for { - var response utils.TSearchResponse - payload := map[string]interface{}{ - "size": pageSize, - "page": page, - "order": "asc", - "type": "IMAGE", - "withStacked": true, - "withDeleted": true, - } - if vis != nil { - payload["visibility"] = vis - } - if err := c.doRequestWithUpstreamRetry(http.MethodPost, "/search/metadata", payload, &response, 4); err != nil { - return nil, fmt.Errorf("error fetching asset IDs (visibility=%v): %w", vis, err) - } - for _, asset := range response.Assets.Items { - if seen[asset.ID] { - continue + for _, assetType := range c.assetTypesForSearch() { + for _, vis := range visibilityFilters { + page := 1 + const pageSize = 1000 + for { + var response utils.TSearchResponse + payload := map[string]interface{}{ + "size": pageSize, + "page": page, + "order": "asc", + "type": assetType, + "withStacked": true, + "withDeleted": true, } - seen[asset.ID] = true - ids = append(ids, asset.ID) - } - if response.Assets.NextPage == "" || response.Assets.NextPage == "0" { - break - } - nextPageInt, err := strconv.Atoi(response.Assets.NextPage) - if err != nil || nextPageInt == 0 { - break + if vis != nil { + payload["visibility"] = vis + } + if err := c.doRequestWithUpstreamRetry(http.MethodPost, "/search/metadata", payload, &response, 4); err != nil { + return nil, fmt.Errorf("error fetching asset IDs (type=%s, visibility=%v): %w", assetType, vis, err) + } + for _, asset := range response.Assets.Items { + if seen[asset.ID] { + continue + } + seen[asset.ID] = true + ids = append(ids, asset.ID) + } + if response.Assets.NextPage == "" || response.Assets.NextPage == "0" { + break + } + nextPageInt, err := strconv.Atoi(response.Assets.NextPage) + if err != nil || nextPageInt == 0 { + break + } + page = nextPageInt } - page = nextPageInt } } return ids, nil From 13840f46f3be1806108dbe56380089d8e203e5eb Mon Sep 17 00:00:00 2001 From: Major Date: Tue, 26 May 2026 19:40:08 +0200 Subject: [PATCH 2/6] add --include-videos flag and INCLUDE_VIDEOS env var 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 --- cmd/config.go | 8 ++++++++ cmd/main.go | 1 + 2 files changed, 9 insertions(+) diff --git a/cmd/config.go b/cmd/config.go index e099ce0..d06326a 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -35,6 +35,7 @@ var withDeleted bool var logLevel string var logFormat string var removeSingleAssetStacks bool +var includeVideos bool var filterAlbumIDs []string var filterTakenAfter string var filterTakenBefore string @@ -153,6 +154,7 @@ func logStartupSummary(logger *logrus.Logger) { "withArchived": withArchived, "withDeleted": withDeleted, "removeSingleAssetStacks": removeSingleAssetStacks, + "includeVideos": includeVideos, "criteria": criteria, "parentFilenamePromote": parentFilenamePromote, "parentExtPromote": parentExtPromote, @@ -197,6 +199,9 @@ func logStartupSummary(logger *logrus.Logger) { if removeSingleAssetStacks { summary = append(summary, "remove-single=true") } + if includeVideos { + summary = append(summary, "include-videos=true") + } if criteria != "" { summary = append(summary, fmt.Sprintf("criteria=%s", criteria)) } @@ -289,6 +294,9 @@ func LoadEnvForTesting() LoadEnvConfig { if !removeSingleAssetStacks { removeSingleAssetStacks = os.Getenv("REMOVE_SINGLE_ASSET_STACKS") == "true" } + if !includeVideos { + includeVideos = os.Getenv("INCLUDE_VIDEOS") == "true" + } if parentFilenamePromote == "" || parentFilenamePromote == utils.DefaultParentFilenamePromoteString { if envVal := os.Getenv("PARENT_FILENAME_PROMOTE"); envVal != "" { parentFilenamePromote = envVal diff --git a/cmd/main.go b/cmd/main.go index d7a4cb4..197a123 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -32,6 +32,7 @@ func bindFlags(rootCmd *cobra.Command) { rootCmd.PersistentFlags().StringVar(&logLevel, "log-level", "", "Log level: debug, info, warn, error (or set LOG_LEVEL env var)") rootCmd.PersistentFlags().StringVar(&logFormat, "log-format", "", "Log format: text, json (or set LOG_FORMAT env var)") rootCmd.PersistentFlags().BoolVar(&removeSingleAssetStacks, "remove-single-asset-stacks", false, "Remove stacks with only one asset (or set REMOVE_SINGLE_ASSET_STACKS=true)") + rootCmd.PersistentFlags().BoolVar(&includeVideos, "include-videos", false, "Include VIDEO assets alongside IMAGE in stacking (or set INCLUDE_VIDEOS=true)") rootCmd.PersistentFlags().StringSliceVar(&filterAlbumIDs, "filter-album-ids", nil, "Filter by album IDs or names, comma-separated (or set FILTER_ALBUM_IDS env var)") rootCmd.PersistentFlags().StringVar(&filterTakenAfter, "filter-taken-after", "", "Filter assets taken after date, ISO 8601 (or set FILTER_TAKEN_AFTER env var)") rootCmd.PersistentFlags().StringVar(&filterTakenBefore, "filter-taken-before", "", "Filter assets taken before date, ISO 8601 (or set FILTER_TAKEN_BEFORE env var)") From 815b282847cce9857a7eaf31cbdc67272f87e66b Mon Sep 17 00:00:00 2001 From: Major Date: Tue, 26 May 2026 19:40:12 +0200 Subject: [PATCH 3/6] wire includeVideos through all command client initializations 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 --- cmd/duplicates.go | 2 +- cmd/fixtrash.go | 2 +- cmd/stacker.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/duplicates.go b/cmd/duplicates.go index 6742341..664e13f 100644 --- a/cmd/duplicates.go +++ b/cmd/duplicates.go @@ -47,7 +47,7 @@ func runDuplicates(cmd *cobra.Command, args []string) { if i > 0 { logger.Infof("\n") } - client := immich.NewClient(apiURL, key, false, false, true, withArchived, withDeleted, false, nil, "", "", logger) + client := immich.NewClient(apiURL, key, false, false, true, withArchived, withDeleted, false, includeVideos, nil, "", "", logger) if client == nil { logger.Errorf("Invalid client for API key: %s", key) continue diff --git a/cmd/fixtrash.go b/cmd/fixtrash.go index ce10feb..2b0bc8c 100644 --- a/cmd/fixtrash.go +++ b/cmd/fixtrash.go @@ -51,7 +51,7 @@ func runFixTrash(cmd *cobra.Command, args []string) { if i > 0 { logger.Infof("\n") } - client := immich.NewClient(apiURL, key, false, false, dryRun, withArchived, withDeleted, false, nil, "", "", logger) + client := immich.NewClient(apiURL, key, false, false, dryRun, withArchived, withDeleted, false, includeVideos, nil, "", "", logger) if client == nil { logger.Errorf("Invalid client for API key: %s", key) continue diff --git a/cmd/stacker.go b/cmd/stacker.go index 6c23c5d..4dbaa10 100644 --- a/cmd/stacker.go +++ b/cmd/stacker.go @@ -174,7 +174,7 @@ func runStacker(cmd *cobra.Command, args []string) { if i > 0 { logger.Infof("\n") } - client := immich.NewClient(apiURL, key, resetStacks, replaceStacks, dryRun, withArchived, withDeleted, removeSingleAssetStacks, filterAlbumIDs, filterTakenAfter, filterTakenBefore, logger) + client := immich.NewClient(apiURL, key, resetStacks, replaceStacks, dryRun, withArchived, withDeleted, removeSingleAssetStacks, includeVideos, filterAlbumIDs, filterTakenAfter, filterTakenBefore, logger) if client == nil { logger.Errorf("Invalid client for API key: %s", key) continue @@ -346,7 +346,7 @@ func runCronLoopForAllUsers(apiKeys []string, apiURL string, logger *logrus.Logg if i > 0 { logger.Infof("\n") } - client := immich.NewClient(apiURL, key, resetStacks, replaceStacks, dryRun, withArchived, withDeleted, removeSingleAssetStacks, filterAlbumIDs, filterTakenAfter, filterTakenBefore, logger) + client := immich.NewClient(apiURL, key, resetStacks, replaceStacks, dryRun, withArchived, withDeleted, removeSingleAssetStacks, includeVideos, filterAlbumIDs, filterTakenAfter, filterTakenBefore, logger) if client == nil { logger.Errorf("Invalid client for API key: %s", key) continue From d385d1200811430d56661e9f7109db77b66a154e Mon Sep 17 00:00:00 2001 From: Major Date: Tue, 26 May 2026 19:40:16 +0200 Subject: [PATCH 4/6] add comprehensive video asset support test coverage 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 --- pkg/immich/client_test.go | 6 +- pkg/immich/client_videos_test.go | 169 +++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 3 deletions(-) create mode 100644 pkg/immich/client_videos_test.go diff --git a/pkg/immich/client_test.go b/pkg/immich/client_test.go index c1f50b3..464fd31 100644 --- a/pkg/immich/client_test.go +++ b/pkg/immich/client_test.go @@ -58,7 +58,7 @@ func TestNewClient(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Act - client := NewClient(tt.apiURL, tt.apiKey, tt.resetStacks, tt.replaceStacks, tt.dryRun, true, false, false, nil, "", "", logrus.New()) + client := NewClient(tt.apiURL, tt.apiKey, tt.resetStacks, tt.replaceStacks, tt.dryRun, true, false, false, false, nil, "", "", logrus.New()) // Assert if tt.wantErr { @@ -892,7 +892,7 @@ func TestNewClientWithFilterParams(t *testing.T) { client := NewClient( "http://test.com", "test-key", - false, false, false, false, false, false, + false, false, false, false, false, false, false, tt.filterAlbumIDs, tt.filterTakenAfter, tt.filterTakenBefore, @@ -1689,7 +1689,7 @@ func TestNewClientEdgeCases(t *testing.T) { client := NewClient( tt.apiURL, tt.apiKey, - false, false, false, false, false, false, + false, false, false, false, false, false, false, nil, "", "", tt.logger, ) diff --git a/pkg/immich/client_videos_test.go b/pkg/immich/client_videos_test.go new file mode 100644 index 0000000..a92d57d --- /dev/null +++ b/pkg/immich/client_videos_test.go @@ -0,0 +1,169 @@ +package immich + +import ( + "encoding/json" + "io" + "net/http" + "strings" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAssetTypesForSearch(t *testing.T) { + tests := []struct { + name string + includeVideos bool + want []string + }{ + {name: "default is IMAGE only", includeVideos: false, want: []string{"IMAGE"}}, + {name: "includeVideos adds VIDEO", includeVideos: true, want: []string{"IMAGE", "VIDEO"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &Client{includeVideos: tt.includeVideos} + assert.Equal(t, tt.want, c.assetTypesForSearch()) + }) + } +} + +/************************************************************************************************** +** typeTrackingTransport captures the "type" field of each /search/metadata POST and counts +** calls per type. Used to verify FetchAssets dispatches the right number of searches based +** on the includeVideos flag. +**************************************************************************************************/ +type typeTrackingTransport struct { + mu sync.Mutex + typesCalls map[string]int +} + +func (t *typeTrackingTransport) RoundTrip(req *http.Request) (*http.Response, error) { + t.mu.Lock() + defer t.mu.Unlock() + if t.typesCalls == nil { + t.typesCalls = make(map[string]int) + } + if strings.HasSuffix(req.URL.Path, "/search/metadata") && req.Body != nil { + var payload map[string]interface{} + body, _ := io.ReadAll(req.Body) + _ = json.Unmarshal(body, &payload) + if assetType, ok := payload["type"].(string); ok { + t.typesCalls[assetType]++ + } + } + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(`{"assets":{"items":[],"nextPage":""}}`)), + }, nil +} + +func TestFetchAssetsRespectsIncludeVideosFlag(t *testing.T) { + tests := []struct { + name string + includeVideos bool + expectedTypeKeys []string + }{ + {name: "default fetches only IMAGE", includeVideos: false, expectedTypeKeys: []string{"IMAGE"}}, + {name: "includeVideos fetches IMAGE and VIDEO", includeVideos: true, expectedTypeKeys: []string{"IMAGE", "VIDEO"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + transport := &typeTrackingTransport{} + client := &Client{ + apiKey: "test", + apiURL: "http://test/api", + logger: newSilentLogger(), + includeVideos: tt.includeVideos, + client: &http.Client{Transport: transport}, + } + _, err := client.FetchAssets(100, nil) + require.NoError(t, err) + + gotTypes := make([]string, 0, len(transport.typesCalls)) + for k := range transport.typesCalls { + gotTypes = append(gotTypes, k) + } + assert.ElementsMatch(t, tt.expectedTypeKeys, gotTypes, + "unexpected /search/metadata type filters actually called: %+v", transport.typesCalls) + }) + } +} + +func TestFetchAllAssetIDsViaSearchRespectsIncludeVideosFlag(t *testing.T) { + // The hybrid fallback enumerates assets via /search/metadata across both visibility + // passes (timeline + archive). With includeVideos, this cross-product should also span + // both asset types — i.e., 4 distinct (type, visibility) combinations. + for _, tt := range []struct { + name string + includeVideos bool + expectedTypeKeys []string + expectedTotalKeys int + }{ + { + name: "default fetches IMAGE × 2 visibilities", + includeVideos: false, + expectedTypeKeys: []string{"IMAGE"}, + expectedTotalKeys: 1, + }, + { + name: "includeVideos fetches IMAGE and VIDEO × 2 visibilities", + includeVideos: true, + expectedTypeKeys: []string{"IMAGE", "VIDEO"}, + expectedTotalKeys: 2, + }, + } { + t.Run(tt.name, func(t *testing.T) { + transport := &typeTrackingTransport{} + client := &Client{ + apiKey: "test", + apiURL: "http://test/api", + logger: newSilentLogger(), + includeVideos: tt.includeVideos, + client: &http.Client{Transport: transport}, + } + _, err := client.fetchAllAssetIDsViaSearch() + require.NoError(t, err) + + gotTypes := make([]string, 0, len(transport.typesCalls)) + for k := range transport.typesCalls { + gotTypes = append(gotTypes, k) + } + assert.ElementsMatch(t, tt.expectedTypeKeys, gotTypes, + "unexpected type filters actually called: %+v", transport.typesCalls) + assert.Len(t, transport.typesCalls, tt.expectedTotalKeys) + }) + } +} + +func TestFetchTrashedAssetsRespectsIncludeVideosFlag(t *testing.T) { + for _, tt := range []struct { + name string + includeVideos bool + expectedTypeKeys []string + }{ + {name: "default fetches only IMAGE trash", includeVideos: false, expectedTypeKeys: []string{"IMAGE"}}, + {name: "includeVideos fetches IMAGE and VIDEO trash", includeVideos: true, expectedTypeKeys: []string{"IMAGE", "VIDEO"}}, + } { + t.Run(tt.name, func(t *testing.T) { + transport := &typeTrackingTransport{} + client := &Client{ + apiKey: "test", + apiURL: "http://test/api", + logger: newSilentLogger(), + includeVideos: tt.includeVideos, + client: &http.Client{Transport: transport}, + } + _, err := client.FetchTrashedAssets(100) + require.NoError(t, err) + + gotTypes := make([]string, 0, len(transport.typesCalls)) + for k := range transport.typesCalls { + gotTypes = append(gotTypes, k) + } + assert.ElementsMatch(t, tt.expectedTypeKeys, gotTypes, + "unexpected /search/metadata type filters actually called: %+v", transport.typesCalls) + }) + } +} From d83d297fb82e4e810cf047125331d4648f3985b2 Mon Sep 17 00:00:00 2001 From: Major Date: Tue, 26 May 2026 19:40:20 +0200 Subject: [PATCH 5/6] document video stacking feature and configuration 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 --- docs/troubleshooting.md | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 5d472c7..4e3d7a2 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -131,6 +131,47 @@ timeline` disabled. This resolves [issue #55](https://github.com/Majorfi/immich-stack/issues/55). +### Stacking Video Files + +**Symptoms:** + +- Stacking criteria match but `.mov`/`.mp4`/other video files are never picked up +- Live Photos (`.HEIC` + `.MOV` pairs) only stack the photo, not the motion file +- Edited videos (trimmed, cropped) cannot be stacked with their originals + +**Cause:** + +By default, immich-stack restricts `/search/metadata` calls to `type=IMAGE`. Video assets +are excluded from the candidate pool entirely, regardless of whether your stacking criteria +would otherwise match them. + +**Solution:** + +Enable `INCLUDE_VIDEOS=true` (or the CLI flag `--include-videos`). When set, every asset +fetch runs twice — once for `IMAGE` and once for `VIDEO` — and results are deduplicated. +Existing stacking criteria (filename patterns, time deltas, regex, etc.) work on videos +the same way they work on images. + +```sh +INCLUDE_VIDEOS=true +``` + +**Examples of use cases this unlocks:** + +- iPhone Live Photos: `IMG_1234.HEIC` paired with `IMG_1234.MOV` (same base name) +- Trimmed/edited videos paired with the original file +- Android burst videos with identical timestamps + +**Performance implications:** + +- A second pagination round runs for VIDEO. Wall-clock latency increases by the time the + VIDEO scan takes — proportional to how many videos you have. A library that's mostly + images sees a small bump; a library with many videos sees more. +- The `OTHER` and `AUDIO` Immich types are still excluded — only `IMAGE` and `VIDEO` are + pulled in. + +This resolves [issue #54](https://github.com/Majorfi/immich-stack/issues/54). + ### Grouping Issues **Symptoms:** From c1d26eed9de1f1d8f25eb88306b44d20466910d1 Mon Sep 17 00:00:00 2001 From: Major Date: Tue, 26 May 2026 20:26:15 +0200 Subject: [PATCH 6/6] address PR review feedback on video support 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. --- cmd/config.go | 7 +++++-- cmd/main.go | 3 +++ cmd/stacker_test.go | 2 ++ pkg/immich/client.go | 1 + pkg/immich/client_videos_test.go | 25 +++++++++++++++++-------- 5 files changed, 28 insertions(+), 10 deletions(-) diff --git a/cmd/config.go b/cmd/config.go index d06326a..bed861d 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -36,6 +36,7 @@ var logLevel string var logFormat string var removeSingleAssetStacks bool var includeVideos bool +var includeVideosFlagSet bool var filterAlbumIDs []string var filterTakenAfter string var filterTakenBefore string @@ -294,8 +295,10 @@ func LoadEnvForTesting() LoadEnvConfig { if !removeSingleAssetStacks { removeSingleAssetStacks = os.Getenv("REMOVE_SINGLE_ASSET_STACKS") == "true" } - if !includeVideos { - includeVideos = os.Getenv("INCLUDE_VIDEOS") == "true" + if !includeVideosFlagSet { + if envInclude := os.Getenv("INCLUDE_VIDEOS"); envInclude != "" { + includeVideos = envInclude == "true" + } } if parentFilenamePromote == "" || parentFilenamePromote == utils.DefaultParentFilenamePromoteString { if envVal := os.Getenv("PARENT_FILENAME_PROMOTE"); envVal != "" { diff --git a/cmd/main.go b/cmd/main.go index 197a123..b26f1ac 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -83,6 +83,9 @@ func CreateRootCommand() *cobra.Command { if cmd.Flags().Lookup("replace-stacks") != nil && cmd.Flags().Lookup("replace-stacks").Changed { replaceStacksFlagSet = true } + if cmd.Flags().Lookup("include-videos") != nil && cmd.Flags().Lookup("include-videos").Changed { + includeVideosFlagSet = true + } }, } diff --git a/cmd/stacker_test.go b/cmd/stacker_test.go index 2c9cc9f..154814e 100644 --- a/cmd/stacker_test.go +++ b/cmd/stacker_test.go @@ -37,6 +37,8 @@ func resetGlobalConfig() { withDeleted = false logLevel = "" removeSingleAssetStacks = false + includeVideos = false + includeVideosFlagSet = false } func clearEnvironment() { diff --git a/pkg/immich/client.go b/pkg/immich/client.go index 314e7ae..36c65b3 100644 --- a/pkg/immich/client.go +++ b/pkg/immich/client.go @@ -90,6 +90,7 @@ type Client struct { ** @param withArchived - Whether to include archived assets ** @param withDeleted - Whether to include deleted assets ** @param removeSingleAssetStacks - Whether to remove stacks with only one asset +** @param includeVideos - Whether to include VIDEO assets alongside IMAGE in searches ** @param filterAlbumIDs - Filter by album IDs (empty slice means no filter) ** @param filterTakenAfter - Filter assets taken after this date (empty means no filter) ** @param filterTakenBefore - Filter assets taken before this date (empty means no filter) diff --git a/pkg/immich/client_videos_test.go b/pkg/immich/client_videos_test.go index a92d57d..c8ba525 100644 --- a/pkg/immich/client_videos_test.go +++ b/pkg/immich/client_videos_test.go @@ -40,19 +40,28 @@ type typeTrackingTransport struct { } func (t *typeTrackingTransport) RoundTrip(req *http.Request) (*http.Response, error) { - t.mu.Lock() - defer t.mu.Unlock() - if t.typesCalls == nil { - t.typesCalls = make(map[string]int) - } + // Decode the asset type filter OUTSIDE the lock — I/O and JSON parsing are slow and + // holding the mutex through them would serialize concurrent requests unnecessarily. + var assetType string if strings.HasSuffix(req.URL.Path, "/search/metadata") && req.Body != nil { - var payload map[string]interface{} body, _ := io.ReadAll(req.Body) + _ = req.Body.Close() + var payload map[string]interface{} _ = json.Unmarshal(body, &payload) - if assetType, ok := payload["type"].(string); ok { - t.typesCalls[assetType]++ + if v, ok := payload["type"].(string); ok { + assetType = v } } + + if assetType != "" { + t.mu.Lock() + if t.typesCalls == nil { + t.typesCalls = make(map[string]int) + } + t.typesCalls[assetType]++ + t.mu.Unlock() + } + return &http.Response{ StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(`{"assets":{"items":[],"nextPage":""}}`)),