feat: add full-resolution image option for asset loading#652
Conversation
📝 WalkthroughWalkthroughThis PR introduces a new ChangesFull-Resolution Image Configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ImmichFrame.Core/Logic/PooledImmichFrameLogic.cs (1)
125-147:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDownload cache is not invalidated when the resolution setting changes.
The on-disk cache lookup (Lines 125-140) and the saved filename (
{id}.{ext}, Line 159) are keyed only on the asset id, not on the requested media size. WhenDownloadImagesis enabled and a user later togglesUseFullResolutionImages, previously cached preview images (or vice-versa) will continue to be served from disk untilRenewImagesDurationelapses, so the setting won't take effect for already-cached assets. Consider encoding the resolution into the cache key/filename so the two variants are stored separately.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ImmichFrame.Core/Logic/PooledImmichFrameLogic.cs` around lines 125 - 147, The cache key currently uses only the asset id, so cached images don't vary by resolution; update the lookup and save logic in PooledImmichFrameLogic to include the requested media size (mediaSize / _generalSettings.UseFullResolutionImages or AssetMediaSize) in the filename/key (e.g., incorporate mediaSize name or flag alongside id), so Directory.GetFiles(...) and Path.GetFileNameWithoutExtension(...) match the same "{id}-{mediaSize}.{ext}" pattern and cache invalidation (RenewImagesDuration, File.Delete) operates on the resolution-specific file; ensure the code paths around mediaSize, ViewAssetAsync, and where the file is written use the same naming convention.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@ImmichFrame.Core/Logic/PooledImmichFrameLogic.cs`:
- Around line 125-147: The cache key currently uses only the asset id, so cached
images don't vary by resolution; update the lookup and save logic in
PooledImmichFrameLogic to include the requested media size (mediaSize /
_generalSettings.UseFullResolutionImages or AssetMediaSize) in the filename/key
(e.g., incorporate mediaSize name or flag alongside id), so
Directory.GetFiles(...) and Path.GetFileNameWithoutExtension(...) match the same
"{id}-{mediaSize}.{ext}" pattern and cache invalidation (RenewImagesDuration,
File.Delete) operates on the resolution-specific file; ensure the code paths
around mediaSize, ViewAssetAsync, and where the file is written use the same
naming convention.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 99cb6eb1-0489-4e72-9d46-2fd8a8705f7a
📒 Files selected for processing (13)
ImmichFrame.Core/Interfaces/IServerSettings.csImmichFrame.Core/Logic/PooledImmichFrameLogic.csImmichFrame.WebApi.Tests/Resources/TestV1.jsonImmichFrame.WebApi.Tests/Resources/TestV2.jsonImmichFrame.WebApi.Tests/Resources/TestV2.ymlImmichFrame.WebApi/Helpers/Config/ServerSettingsV1.csImmichFrame.WebApi/Models/ServerSettings.csInstall_Web.mddocker/Settings.example.jsondocker/Settings.example.ymldocker/example.envdocs/docs/getting-started/configuration.mddocs/docs/getting-started/configurationV1.md
1 participant
Added new global setting UseFullResolutionImages with default false to preserve current behavior.
Updated image loading logic to request fullsize assets when the flag is enabled, otherwise keep using preview.
Summary by CodeRabbit
Release Notes
New Features
Documentation