Fix: WhereExcludes is O(n*m) and stalls the asset pool for large excluded albums

[sheltonial]

Problem

WhereExcludes rescans the entire excluded collection for every item in the source:

public static IEnumerable<T> WhereExcludes<T>(this IEnumerable<T> source, IEnumerable<T> excluded, Func<T, object> comparator)
    => source.Where(item1 => !excluded.Any(item2 => Equals(comparator(item2), comparator(item1))));

That is O(n×m), and allocation-heavy with it: the Func<T, object> comparator boxes a Guid on every single comparison, on both sides.

It has one call site, AssetExtensionMethods.ApplyAccountFilters:

assets = assets.WhereExcludes(excludedAlbumAssets, t => t.Id);

and CachingApiAssetsPool caches the resulting lazy IEnumerable rather than a materialised list, so the full scan re-runs on every enumeration of the pool.

With a small ExcludedAlbums this is invisible. On my library it isn't:

• n = 30,313 assets across four configured People
• m = 3,618 assets in an excluded album — videos over 30s, collected there so ShowVideos could be turned on without a 20-minute clip parking the frame for 20 minutes

That works out to ~5.5×10⁷ comparisons per enumeration, several enumerations per request. Authenticated /api/Asset requests stopped returning entirely — they hung past a 60s timeout, so the frame served nothing rather than erroring, which made it look like an empty pool rather than a slow one. Dropping back to the 98-asset excluded album I had before (~1.5×10⁶ comparisons) made it instant again.

Fix

Hash the excluded keys once, then test membership:

var excludedKeys = excluded.Select(comparator).ToHashSet();

return source.Where(item => !excludedKeys.Contains(comparator(item)));

O(n+m), and boxing drops from one per comparison to one per item. TagAssetsPool already uses a HashSet<Guid> for the same kind of lookup.

Semantics are unchanged: EqualityComparer<object>.Default calls the same object.Equals the old code did, and for a boxed Guid that is value equality backed by a consistent GetHashCode. Empty excluded and null elements behave as before.

One deliberate behavioural detail: since the body is no longer a single lazy expression, excluded is enumerated once when WhereExcludes is called, instead of repeatedly on each enumeration of the result. The only call site passes an already-materialised list, and building the set eagerly is precisely what makes repeat enumerations of the cached pool cheap.

Happy to go further and make the comparator generic (Func<T, TKey>) to remove boxing altogether, but that changes the public signature so I left it out of this PR.

Tests

Adds ImmichFrame.Core.Tests/Helpers/CollectionExtensionMethodsTests.cs:

• excludes by comparator value, not instance identity
• empty excluded returns everything
• the result is stable when enumerated twice
• excluded is enumerated once, not once per source item

The last one is the regression guard. On main it fails with Expected: 1, But was: 100; the other three pass either way, which is the intended demonstration that the change is behaviour-preserving.

Existing AllAssetsPoolTests.GetAssets_ExcludesAssetsFromExcludedAlbums and the AlbumAssetsPoolTests exclusion cases cover the end-to-end semantics and are untouched.

Both suites pass locally on .NET 8: ImmichFrame.Core.Tests 88/88 and ImmichFrame.WebApi.Tests 26/26.

Summary by CodeRabbit

• Bug Fixes

  • Improved collection filtering to compare items by their selected keys, producing more reliable exclusion results.
  • Improved efficiency when filtering large collections, reducing unnecessary repeated checks.
  • Ensured repeated enumeration returns consistent results and excluded data is processed only once.

• Tests

  • Added coverage for key-based exclusions, empty exclusion sets, repeated enumeration, and efficient collection processing.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 964a2943-fa8b-4f95-aa2f-dce4ec391e0e

📥 Commits

Reviewing files that changed from the base of the PR and between 5f0c3c3 and b027cf5.

📒 Files selected for processing (2)

• ImmichFrame.Core.Tests/Helpers/CollectionExtensionMethodsTests.cs
• ImmichFrame.Core/Helpers/CollectionExtensionMethods.cs

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

WhereExcludes now creates a HashSet of excluded keys before filtering. New tests verify key-based exclusion, empty input, repeated enumeration, and single enumeration of the excluded collection.

Changes

WhereExcludes filtering

Layer / File(s)	Summary
Hash-set exclusion filtering ImmichFrame.Core/Helpers/CollectionExtensionMethods.cs	WhereExcludes builds a HashSet from comparator results and uses hash lookups to filter the source.
WhereExcludes behavior validation ImmichFrame.Core.Tests/Helpers/CollectionExtensionMethodsTests.cs	Tests verify key-based exclusion, empty exclusions, repeatable results, and one-time enumeration of the excluded collection.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to b027c

The exclusion lookup optimization preserves the demonstrated key-based behavior while avoiding repeated scans. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: improving the O(n*m) performance of WhereExcludes for large excluded albums. It is specific and concise.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[3rob3]

Looks good to me, what do you think @JW-CH ?