feat(photos): harden timeline and native media rendering - #249
Conversation
veryCrunchy
added
area:photos
veryCrunchy
added
area:ux
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28ff2fcc93
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
https://github.com/Obiente/nc-native/blob/630a6690377479f81465d0e7038035bf9f5cbd9f/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/PreviewMemoryCache.kt#L55
When a DAV or Memories record has no ETag, the nullable value becomes part of a stable cache key, so replacing or editing the remote file under the same file ID leaves loadPreviewCached() returning the old bytes for the rest of the process even after timeline refresh. Memories parsing explicitly permits missing ETags, making this reachable for normal records. Bypass the cache when no trustworthy generation exists, or include another authoritative generation and invalidate it on refresh.
AGENTS.md reference: AGENTS.md:L203-L206
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
veryCrunchy
added
bug
ChatGPT Codex Connector
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f353c07139
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return fallback(cursor) | ||
| } | ||
|
|
||
| val afterDayId = cursor?.let(::decodeMemoriesFolderInventoryCursor) |
There was a problem hiding this comment.
When the folder index succeeds but the initial day-content request falls back, loadPage() returns a DAV page and its cursor while retaining the Memories index in cache. The next request reaches this unconditional Memories decoder with that DAV cursor and fails, making every older folder page unreachable. Dispatch non-Memories cursors directly to fallback, or persist the selected paging source for the generation.
AGENTS.md reference: AGENTS.md:L139-L140
Useful? React with 👍 / 👎.
| } else { | ||
| MediaTimelineDavCursorPart(boundaryEpochSeconds = null, firstResult = 0) | ||
| } | ||
| val response = executeOptionalRawSearch(execute, request.body) ?: return pages |
There was a problem hiding this comment.
When optional RAW discovery has already loaded one filename-pattern chunk and a later initial SEARCH throws, this return publishes only the accumulated partitions; the encoded cursor contains no entry for the failed or unattempted patterns, so subsequent pages never retry them and RAW-only originals remain absent until a full refresh. Fresh evidence in this head is that retry cursors are now created only in the decoded-cursor branch via retainRetryCursor(), while this initial-discovery path still returns partial pages. Preserve cursors for the failed and remaining pattern chunks and mark the page retry-pending.
AGENTS.md reference: AGENTS.md:L402-L403
Useful? React with 👍 / 👎.
| require(day.itemCount <= MAX_MEDIA_ITEMS_PER_RESPONSE) { | ||
| "A single Memories folder day exceeds the safe media response limit." |
There was a problem hiding this comment.
When a recursive folder index contains a historical day with more than 10,000 items after one or more smaller windows, earlier requests emit Memories cursors, but the request that reaches that day throws here on every retry. All media older than that day therefore remains unreachable. Inspect the complete index before emitting the first Memories cursor and choose DAV fallback for the generation, or implement a bounded within-day cursor.
AGENTS.md reference: AGENTS.md:L139-L140
Useful? React with 👍 / 👎.
| val response = services.executeNextcloudApi( | ||
| session, | ||
| memoriesPhotoDecodableApiRequest(fileId, etag), | ||
| ) |
There was a problem hiding this comment.
When the fast preview falls through to loadMemoriesDecodableImageCached() for a RAW or uncommon image, that call already downloads and caches the full decodable-image response, but the automatic high-detail phase immediately executes the identical file-ID/ETag request again here instead of consulting that cache. Opening one such item therefore transfers and retains up to 64 MiB twice before the user interacts with it, contrary to the advertised cache reuse and with avoidable data, battery, and memory cost. Reuse the generation-pinned Memories payload from the preview cache or carry it into the high-detail phase.
AGENTS.md reference: AGENTS.md:L25-L26
Useful? React with 👍 / 👎.
| private fun formatMediaEpochSeconds(epochSeconds: Long): String { | ||
| require(epochSeconds >= 0L) | ||
| return epochSeconds.toString() |
There was a problem hiding this comment.
For Memories-backed records when embedded EXIF is unavailable, including every desktop record using the default media-information implementation, the primary Captured field is rendered as a raw value such as 1775000000. Users cannot interpret that as a capture date or time, so format the epoch with the platform-local date, time, and timezone conventions before exposing it in the information dialog.
AGENTS.md reference: AGENTS.md:L64-L66
Useful? React with 👍 / 👎.
| } | ||
| } | ||
| val fileDetails = buildList { | ||
| add(MediaInformationField("path", "Path", path)) |
There was a problem hiding this comment.
For timeline, collection, and recursive-folder records where davPathAuthoritative is false, this field exposes presentation-only values such as memories/collections/... or <fileId>-<name> under the user-facing label Path. Desktop always shows that invented location, and Android does so whenever file-ID resolution fails, misleading users into believing it is the object's real Nextcloud path. Include this field only for an authoritative DAV record, or resolve the file identity before presenting a path.
AGENTS.md reference: AGENTS.md:L154-L157
Useful? React with 👍 / 👎.
1 participant
Outcome
This pass replaces fragile, overlapping photo discovery with Memories-native timeline and folder flows, while keeping bounded WebDAV compatibility paths for servers that need them. It also adds progressive original-quality image loading, RAW/TIFF handling, media information, and a bounded Android video compatibility fallback.
What changed
Root causes addressed
Timeline and folder consumers shared SearchDAV carryover state and could start overlapping requests. Optional RAW enrichment was also allowed to turn a successful ordinary-media load into a fatal screen error. Large or uncommon images depended too heavily on server thumbnails and platform-default decoding, while decoder capability failures did not have a bounded native compatibility path.
Validation
Closes #248
Advances #242
Advances #243
Advances #95
Advances #84
Advances #86
Advances #83
Advances #74
Advances #182