Skip to content

Front-end for subalbums - #4711

Open
ildyria wants to merge 17 commits into
feature-62/bucket-album-approach-tags-owners-rootfrom
feature-63/bucket-album-front-end
Open

Front-end for subalbums#4711
ildyria wants to merge 17 commits into
feature-62/bucket-album-approach-tags-owners-rootfrom
feature-63/bucket-album-front-end

Conversation

@ildyria

@ildyria ildyria commented Sep 2, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • New Features

    • Album galleries now support faster virtualized grid and list views across root, shared, and sub-album galleries.
    • Timeline bucket headers, responsive layouts, lazy cover loading, client-side date formatting, and improved drag selection are supported.
    • Album thumbnails resolve covers more reliably across gallery and search views.
  • Bug Fixes

    • Reduced duplicate account and permissions requests during loading.
    • Prevented unnecessary menu resets during gallery refreshes.
    • Ignored benign browser resize notifications in error reporting.
    • Guests no longer trigger unnecessary user-count requests.

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: c96dab40-784f-4245-b65c-f34bac7e53da

📥 Commits

Reviewing files that changed from the base of the PR and between 338e6da and 86d6639.

📒 Files selected for processing (1)
  • docs/specs/4-architecture/open-questions.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/specs/4-architecture/open-questions.md

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


📝 Walkthrough

Walkthrough

Changes

Album timeline and root-gallery adoption

Layer / File(s) Summary
Feature plan and architecture records
docs/specs/4-architecture/...
Adds Feature 063 plans, tasks, roadmap entries, resolved questions, and corrected merge-conflict placement.
V3 services, adapters, and store loading
resources/js/services/..., resources/js/stores/..., resources/js/v8/utils/...
Adds cached v3 requests, bucket boundaries, tile adaptation, rights merging, date formatting, and flag-gated store loading for subalbums and root categories.
Shared virtual row and album view components
resources/js/v8/composables/album/..., resources/js/v8/components/gallery/albumModule/Virtualized/...
Adds responsive geometry, virtualized grid/list rendering, sticky bucket headers, accessibility metadata, and album interaction propagation.
Root gallery wiring, covers, and selection
resources/js/v8/views/gallery-panels/Albums.vue, resources/js/composables/album/dragAndSelect.ts, resources/js/v8/components/gallery/albumModule/...
Adds root own/shared virtual panels, v3 cover resolution, feature-flag fallbacks, and drag selection across virtualized album grids.
Runtime request and UI support updates
resources/js/services/auth-service.ts, resources/js/services/init-service.ts, resources/js/v8/menus/LeftMenu.vue, resources/js/v8/views/Error.vue, resources/js/v8/...
Coalesces concurrent requests, preserves menu state during refreshes, skips guest user counts, ignores benign resize errors, and updates comments and metadata.

Estimated code review effort: 5 (Critical) | ~120 minutes

Merge Risk: 🟡 Moderate · up to 86d66

The subalbum front end can show incorrectly scoped shared albums and has several selection and interaction defects; unresolved conflict markers and inconsistent feature records also leave the change not ready to merge. Resolve these issues before merging.

Poem

A rabbit hops through buckets bright
Tiles align in measured rows
Covers peek from caches deep
Sticky headers mark the flow
V3 paths now neatly meet
Carrots celebrate the change

🚥 Pre-merge checks | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.61% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 27 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Full details: Docstring Coverage

Explanation

Docstring coverage is 60.61% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 27 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI

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

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

@ildyria
ildyria marked this pull request as ready for review September 4, 2026 16:34
@ildyria
ildyria requested a review from a team as a code owner September 4, 2026 16:34
@ildyria

ildyria commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review please.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 20

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
resources/js/stores/AlbumsState.ts (1)

63-63: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Include V3 shared tiles in selectableAlbums.

sharedAlbumsV3 is excluded from this getter. Under struct-of-array mode, shared albums cannot participate in operations that use selectableAlbums, and hasHidden does not inspect them.

Proposed fix
- return state.pinnedAlbums.concat(state.albums.concat(state.sharedAlbums.map((album) => album.data).flat())).filter((a) => {
+ return state.pinnedAlbums.concat(state.albums, state.sharedAlbumsV3, state.sharedAlbums.map((album) => album.data).flat()).filter((a) => {
docs/specs/4-architecture/features/063-album-timeline-buckets-adoption/plan.md (1)

251-251: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add the required Markdown footer to both documents.

Append this content after the final document section:

---

*Last updated: 2026-09-04*
  • docs/specs/4-architecture/features/063-album-timeline-buckets-adoption/plan.md#L251-L251: append the footer after the follow-ups.
  • docs/specs/4-architecture/features/063-album-timeline-buckets-adoption/tasks.md#L261-L261: append the footer after the notes.

As per coding guidelines, documentation files must end with a horizontal rule and the update date.

Source: Coding guidelines

🧹 Nitpick comments (1)
docs/specs/4-architecture/features/063-album-timeline-buckets-adoption/tasks.md (1)

76-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Align the breakpoint notes with the active v8 model.

resources/sass/app-v8.css defines 3xl and 4xl thresholds, and albumTileWidth.ts models them with matching formulas (plus 5xl and 6xl). Remove the stale “dead” and “five breakpoints” notes from the task, plan, and open-question entries. Keep the 3xl/4xl implementation and verification requirements.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: e9590f93-d9b5-41c1-bd13-76622d7612dc

📥 Commits

Reviewing files that changed from the base of the PR and between 52d8f20 and 338e6da.

📒 Files selected for processing (56)
  • docs/specs/4-architecture/features/063-album-timeline-buckets-adoption/plan.md
  • docs/specs/4-architecture/features/063-album-timeline-buckets-adoption/spec.md
  • docs/specs/4-architecture/features/063-album-timeline-buckets-adoption/tasks.md
  • docs/specs/4-architecture/open-questions.md
  • docs/specs/4-architecture/roadmap.md
  • resources/js/app-v8.ts
  • resources/js/composables/album/dragAndSelect.ts
  • resources/js/composables/toast-contract.ts
  • resources/js/router/paths.ts
  • resources/js/services/album-category-v3-service.ts
  • resources/js/services/album-children-v3-service.ts
  • resources/js/services/album-service.ts
  • resources/js/services/auth-service.ts
  • resources/js/services/init-service.ts
  • resources/js/stores/AlbumState.ts
  • resources/js/stores/AlbumsState.ts
  • resources/js/v7/views/BulkAlbumEdit.vue
  • resources/js/v8/components/drawers/AlbumEdit.vue
  • resources/js/v8/components/forms/album/SearchTargetAlbum.vue
  • resources/js/v8/components/gallery/albumModule/AlbumListItem.vue
  • resources/js/v8/components/gallery/albumModule/AlbumPanel.vue
  • resources/js/v8/components/gallery/albumModule/AlbumThumbPanel.vue
  • resources/js/v8/components/gallery/albumModule/Virtualized/AlbumListItemVirtual.vue
  • resources/js/v8/components/gallery/albumModule/Virtualized/AlbumListViewVirtual.vue
  • resources/js/v8/components/gallery/albumModule/Virtualized/AlbumRootGridVirtual.vue
  • resources/js/v8/components/gallery/albumModule/Virtualized/AlbumRootListViewVirtual.vue
  • resources/js/v8/components/gallery/albumModule/Virtualized/AlbumRootPanelVirtual.vue
  • resources/js/v8/components/gallery/albumModule/Virtualized/AlbumThumbGridVirtual.vue
  • resources/js/v8/components/gallery/albumModule/Virtualized/AlbumThumbPanelVirtual.vue
  • resources/js/v8/components/gallery/albumModule/Virtualized/AlbumThumbVirtual.vue
  • resources/js/v8/components/gallery/albumModule/thumbs/AlbumThumb.vue
  • resources/js/v8/components/gallery/albumModule/thumbs/AlbumThumbImage.vue
  • resources/js/v8/components/modals/SpotlightSearch.vue
  • resources/js/v8/components/thumbs/Thumb.vue
  • resources/js/v8/composables/album/albumTileWidth.ts
  • resources/js/v8/composables/album/virtualAlbumRows.ts
  • resources/js/v8/composables/landing/useLandingAnimation.ts
  • resources/js/v8/composables/spotlight/useSpotlightGalleryActions.ts
  • resources/js/v8/composables/useAppToast.ts
  • resources/js/v8/composables/useConfirmDialog.ts
  • resources/js/v8/composables/useScrollReveal.ts
  • resources/js/v8/icons.ts
  • resources/js/v8/menus/LeftMenu.vue
  • resources/js/v8/router/routes.ts
  • resources/js/v8/services/track-service.ts
  • resources/js/v8/utils/adaptAlbumChildTile.ts
  • resources/js/v8/utils/adaptCategoryTile.ts
  • resources/js/v8/utils/albumBucketBoundaries.ts
  • resources/js/v8/utils/aspectRatioNumber.ts
  • resources/js/v8/utils/phpDateFormat.ts
  • resources/js/v8/utils/resolveCssLengthPx.ts
  • resources/js/v8/views/BulkAlbumEdit.vue
  • resources/js/v8/views/Error.vue
  • resources/js/v8/views/gallery-panels/Albums.vue
  • resources/js/v8/views/gallery-panels/Map.vue
  • resources/sass/app-v8.css

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


function dayOfYear(date: Date): number {
const start = new Date(date.getFullYear(), 0, 1);
return Math.floor((date.getTime() - start.getTime()) / 86400000);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🌐 Web query:

In ECMAScript, can subtracting two local-midnight Date values across a daylight-saving transition produce a non-integral number of 24-hour days?

💡 Result:

Yes, in ECMAScript, subtracting two local-midnight Date values across a daylight-saving time (DST) transition can produce a non-integral number of 24-hour days [1][2][3]. When you subtract two Date objects in JavaScript, the operation implicitly converts them to their underlying UTC-based millisecond timestamp values and returns the difference [4][5]. Because ECMAScript Date objects represent local time by applying a system-specific timezone offset and daylight saving adjustment to a UTC base [6][7][8], the number of milliseconds between two "local midnight" moments is not necessarily a multiple of 86,400,000 milliseconds (the number of milliseconds in a standard 24-hour day) [2][3]. If a DST transition occurs between the two dates (e.g., a "spring forward" or "fall back" event), the local timezone offset changes [9][10]. Consequently, the elapsed wall-clock time between two consecutive local midnights will be 23 hours, 25 hours, or some other duration depending on the specific timezone rules [2]. When this millisecond difference is divided by the standard constant for a 24-hour day, the result is a non-integral number of days [1][3]. To perform calendar-based arithmetic that ignores DST and treats "days" as calendar dates, developers typically use UTC-based methods (e.g., Date.UTC) to bypass local timezone offsets [1][2] or rely on specialized libraries designed to handle DST-aware duration calculations [11][9].

Citations:


Calculate z from calendar dates.

When the local time zone crosses a daylight-saving transition, the elapsed time between January 1 and date can be 23 or 25 hours for a calendar day. Math.floor(... / 86400000) can therefore return an incorrect day-of-year value. Use UTC calendar components for this calculation.

Proposed fix
 function dayOfYear(date: Date): number {
-	const start = new Date(date.getFullYear(), 0, 1);
-	return Math.floor((date.getTime() - start.getTime()) / 86400000);
+	const start = Date.UTC(date.getFullYear(), 0, 1);
+	const current = Date.UTC(date.getFullYear(), date.getMonth(), date.getDate());
+	return Math.floor((current - start) / 86400000);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return Math.floor((date.getTime() - start.getTime()) / 86400000);
function dayOfYear(date: Date): number {
const start = Date.UTC(date.getFullYear(), 0, 1);
const current = Date.UTC(date.getFullYear(), date.getMonth(), date.getDate());
return Math.floor((current - start) / 86400000);
}

Comment on lines +209 to +212
default:
// Unrecognized character: pass through literally, matching
// PHP's own date() behavior for an unknown format character.
result += char;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🔎 Supported by static analysis

🌐 Web query:

What format characters does PHP date() recognize, including ISO week-year, timezone, and daylight-saving tokens?

💡 Result:

The PHP date function—and the shared DateTimeInterface::format method—recognize a specific set of format characters to represent date and time components [1][2][3]. ISO Week-Year and Week Tokens To correctly handle ISO 8601 week dates, PHP provides the following characters [3][4]: - W: ISO 8601 week number of year (weeks starting on Monday; 01 through 53) [3]. - o: ISO 8601 week-numbering year. This is similar to Y, but if the ISO week number (W) belongs to the previous or next year, that year is used instead [3]. Timezone Tokens The following tokens are used for timezone information [5][2]: - e: Timezone identifier (e.g., UTC, Atlantic/Azores) [2]. - I (uppercase i): Whether or not the date is in daylight saving time (1 if it is, 0 otherwise) [2]. - O: Difference to Greenwich time (GMT) in hours and minutes (e.g., +0200) [2]. - P: Difference to Greenwich time (GMT) with a colon between hours and minutes (e.g., +02:00) [2]. - p: The same as P, but returns Z for UTC (e.g., +02:00 or Z) [2]. - T: Timezone abbreviation (e.g., UTC, EST, MDT) [5][2]. - Z: Timezone offset in seconds. The offset for timezones west of UTC is always negative, and for those east of UTC is always positive (e.g., -43200 through 50400) [2]. Other Common Format Characters - Day: d (01-31), D (Mon-Sun), j (1-31), l (full weekday name), N (1 for Monday-7 for Sunday), S (ordinal suffix), w (0 for Sunday-6 for Saturday), z (0-365) [2][3]. - Month: F (full name), m (01-12), M (Jan-Dec), n (1-12), t (days in month) [2][3]. - Year: L (leap year check: 1 or 0), Y (4-digit year), y (2-digit year), X/x (expanded year representations) [2][3]. - Time: a/A (am/pm), B (Swatch Internet time), g/G (12/24-hour, no leading zero), h/H (12/24-hour, with leading zero), i (minutes), s (seconds), u (microseconds), v (milliseconds) [2][6][3]. Important Implementation Notes - Escape Characters: Any recognized character can be prevented from being expanded by preceding it with a backslash () [1][5]. - Unix Timestamps: The date function operates on integer Unix timestamps, which do not inherently include timezone information [5][7]. To format timezone-aware dates, you should use the DateTimeImmutable class and its format method instead [7].

Citations:


🤖 get_repo_knowledge executed:

get_repo_knowledge LycheeOrg/Lychee /tmp/coderabbit-repo-knowledge/lycheeorg-lychee-cc271fdf/conventions /tmp/coderabbit-repo-knowledge/lycheeorg-lychee-cc271fdf/learnings

Length of output: 32004


🏁 Script executed:

#!/bin/bash
set -eu
file="resources/js/v8/utils/phpDateFormat.ts"
printf '%s\n' '--- target file ---'
cat -n "$file" | sed -n '1,280p'
printf '%s\n' '--- direct references ---'
rg -n --glob '!vendor' --glob '!node_modules' 'phpDateFormat|formatMinMaxDate|date_format|dateFormat|date format' resources app config tests 2>/dev/null | head -200

Repository: LycheeOrg/Lychee

Length of output: 23053


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- configuration definitions ---'
rg -n -C 8 --glob '*.php' "date_format_album_thumb|date_format_hero_created_at|date_format_hero_min_max|date_format_flow_min_max" config app database resources | head -240
printf '%s\n' '--- server formatting path ---'
cat -n app/Http/Resources/Models/ThumbAlbumResource.php | sed -n '45,145p'
printf '%s\n' '--- client call path ---'
cat -n resources/js/v8/utils/adaptAlbumChildTile.ts | sed -n '125,175p'

Repository: LycheeOrg/Lychee

Length of output: 32736


Support the PHP format tokens used by date_format_album_thumb.

phpDateFormat() emits omitted recognized tokens such as o, O, and T literally. The date_format_album_thumb setting accepts free-text PHP date formats, while ThumbAlbumResource formats the same setting with PHP. Valid formats using these tokens can therefore produce incorrect v8 album metadata. Support the configured PHP token set or reject unsupported tokens.

Comment on lines +251 to +252
const minFormatted = phpDateFormat(format, new Date(minTakenAt));
const maxFormatted = phpDateFormat(format, new Date(maxTakenAt));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
set -euo pipefail

fd -t f 'ThumbAlbumResource.php' app
fd -t f 'AlbumDataResource.php' app
rg -n -C 8 'formatMinMaxDate|min_taken_at|max_taken_at|date_format_album_thumb|timezone' app

Repository: LycheeOrg/Lychee

Length of output: 50373


🤖 get_repo_knowledge executed:

get_repo_knowledge LycheeOrg/Lychee /tmp/coderabbit-repo-knowledge/lycheeorg-lychee-cc271fdf/learnings /tmp/coderabbit-repo-knowledge/lycheeorg-lychee-cc271fdf/conventions

Length of output: 13682


🏁 Script executed:

#!/bin/bash
set -euo pipefail

resource=$(fd -t f 'ThumbAlbumResource.php' app | head -n 1)
printf '%s\n' "== $resource =="
cat -n "$resource"

printf '%s\n' '== date-format call sites and definitions =='
rg -n -C 5 'formatMinMaxDate|phpDateFormat|date_format_album_thumb|min_taken_at|max_taken_at' resources/js app/Http/Resources app/Actions/Album

Repository: LycheeOrg/Lychee

Length of output: 50372


🏁 Script executed:

#!/bin/bash
set -euo pipefail

resource=$(fd -t f 'ThumbAlbumResource.php' app | head -n 1)
cat -n "$resource"

printf '%s\n' '== relevant JavaScript references =='
rg -n -C 6 'formatMinMaxDate|phpDateFormat|date_format_album_thumb|min_taken_at|max_taken_at' resources/js

Repository: LycheeOrg/Lychee

Length of output: 29051


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== V3 producer =='
cat -n app/Actions/Album/StructOfArrays/BuildAlbumDataResource.php | sed -n '60,160p'
printf '%s\n' '== V3 resource =='
cat -n app/Http/Resources/V3/AlbumDataResource.php | sed -n '45,90p'
printf '%s\n' '== client formatter =='
cat -n resources/js/v8/utils/phpDateFormat.ts | sed -n '90,110p;219,260p'
printf '%s\n' '== UTC model contracts and configuration =='
rg -n -C 5 'DB_TIMEZONE_NAME|date_default_timezone_set|app.*timezone|timezone.*app|class PatchedBaseModel|HasUTCBasedTimes' app config bootstrap

Repository: LycheeOrg/Lychee

Length of output: 40294


Parse album timestamps in the server timezone. The V3 resource passes raw UTC database strings to formatMinMaxDate(). new Date() interprets timezone-less date-time strings in the visitor’s local timezone, while ThumbAlbumResource formats the corresponding Carbon values in the application timezone. Date ranges near a day, month, or year boundary can differ. Add an explicit timezone to the client parsing or send timezone-qualified timestamps.

Comment on lines +55 to +62
<AlbumRootPanelVirtual
v-if="is_struct_of_array_enabled"
scope="own"
header="gallery.albums"
:selected-albums="selectedAlbumsIds"
@clicked="albumSelect"
@contexted="contextMenuAlbumOpen"
/>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Forward selected from every virtual root panel.

AlbumRootPanelVirtual relays selected, but these call sites do not handle it. In touch selection mode, AlbumListItemVirtual emits only selected, so root albums cannot be selected.

  • resources/js/v8/views/gallery-panels/Albums.vue#L55-L62: add @selected="albumSelect".
  • resources/js/v8/views/gallery-panels/Albums.vue#L76-L82: add @selected="albumSelect".
  • resources/js/v8/views/gallery-panels/Albums.vue#L111-L118: add @selected="albumSelect".
  • resources/js/v8/views/gallery-panels/Albums.vue#L130-L136: add @selected="albumSelect".
📍 Affects 1 file
  • resources/js/v8/views/gallery-panels/Albums.vue#L55-L62 (this comment)
  • resources/js/v8/views/gallery-panels/Albums.vue#L76-L82
  • resources/js/v8/views/gallery-panels/Albums.vue#L111-L118
  • resources/js/v8/views/gallery-panels/Albums.vue#L130-L136

Comment on lines +443 to +447
if (is_struct_of_array_enabled.value) {
if (sharedAlbumsVisibilityMode.value === "separate_shared_only") {
return albumsStore.sharedAlbumsV3.some((album) => !album.is_public);
}
return albumsStore.sharedAlbumsV3.length > 0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Do not render public albums in separate_shared_only mode.

This condition only decides whether to show the shared panel. AlbumRootPanelVirtual still renders the full sharedAlbumsV3 collection. Therefore, struct-of-array mode shows public albums that the legacy displaySharedAlbums path removes.

Pass a filtered V3 collection to the virtual panel and recompute its bucket boundaries, or make the V3 endpoint return the required subset.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Changed files ==='
git diff --name-status HEAD^ HEAD

echo '=== Conflict markers ==='
rg -n '^(<<<<<<<|=======|>>>>>>>)' . \
  -g '!vendor/**' -g '!node_modules/**' || true

echo '=== Diff stat ==='
git diff --stat HEAD^ HEAD

echo '=== Key module outlines ==='
for f in \
  resources/js/stores/AlbumState.ts \
  resources/js/stores/AlbumsState.ts \
  resources/js/composables/album/dragAndSelect.ts \
  resources/js/v8/composables/album/virtualAlbumRows.ts \
  resources/js/v8/utils/albumBucketBoundaries.ts \
  resources/js/v8/composables/album/albumTileWidth.ts \
  resources/js/v8/utils/adaptAlbumChildTile.ts \
  resources/js/v8/utils/adaptCategoryTile.ts
do
  if [ -f "$f" ]; then
    echo "--- $f ---"
    ast-grep outline "$f" --items all || true
  fi
done

echo '=== High-risk implementation diff ==='
git diff --unified=60 HEAD^ HEAD -- \
  resources/js/stores/AlbumState.ts \
  resources/js/stores/AlbumsState.ts \
  resources/js/composables/album/dragAndSelect.ts \
  resources/js/v8/composables/album/virtualAlbumRows.ts \
  resources/js/v8/utils/albumBucketBoundaries.ts \
  resources/js/v8/composables/album/albumTileWidth.ts \
  resources/js/v8/utils/adaptAlbumChildTile.ts \
  resources/js/v8/utils/adaptCategoryTile.ts

echo '=== Virtual component diff ==='
git diff --unified=35 HEAD^ HEAD -- \
  resources/js/v8/components/gallery/albumModule/Virtualized \
  resources/js/v8/components/gallery/albumModule/AlbumPanel.vue \
  resources/js/v8/views/gallery-panels/Albums.vue

Length of output: 17333


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Refs and merge base ==='
git branch -a --no-color
git remote -v
git merge-base HEAD origin/feature-62/bucket-album-approach-tags-owners-root || true
BASE="$(git merge-base HEAD origin/feature-62/bucket-album-approach-tags-owners-root)"
printf 'BASE=%s\n' "$BASE"

echo '=== Complete PR changed files ==='
git diff --name-status "$BASE"...HEAD
git diff --stat "$BASE"...HEAD

echo '=== Complete PR conflict-marker diff and provenance ==='
git diff --unified=12 "$BASE"...HEAD -- docs/specs/4-architecture/open-questions.md docs/specs/_current-session.md
git blame -L 160,225 -- docs/specs/4-architecture/open-questions.md || true
git blame -L 560,730 -- docs/specs/4-architecture/open-questions.md || true

echo '=== Store and utility implementations ==='
for f in \
  resources/js/stores/AlbumState.ts \
  resources/js/stores/AlbumsState.ts \
  resources/js/composables/album/dragAndSelect.ts \
  resources/js/v8/composables/album/virtualAlbumRows.ts \
  resources/js/v8/utils/albumBucketBoundaries.ts \
  resources/js/v8/utils/adaptAlbumChildTile.ts \
  resources/js/v8/utils/adaptCategoryTile.ts
do
  echo "--- $f ---"
  cat -n "$f"
done

echo '=== Services and virtualized component outlines ==='
for f in \
  resources/js/services/album-category-v3-service.ts \
  resources/js/services/album-children-v3-service.ts \
  resources/js/services/album-service.ts \
  resources/js/v8/components/gallery/albumModule/Virtualized/AlbumThumbGridVirtual.vue \
  resources/js/v8/components/gallery/albumModule/Virtualized/AlbumRootGridVirtual.vue \
  resources/js/v8/components/gallery/albumModule/Virtualized/AlbumListViewVirtual.vue \
  resources/js/v8/components/gallery/albumModule/Virtualized/AlbumRootListViewVirtual.vue \
  resources/js/v8/components/gallery/albumModule/Virtualized/AlbumThumbVirtual.vue
do
  echo "--- $f ---"
  ast-grep outline "$f" --items all || true
done

Length of output: 2653


🏁 Script executed:

#!/bin/bash
set -u

REPO="LycheeOrg/Lychee"
PR="4711"

echo '=== Pull request metadata ==='
gh api "repos/${REPO}/pulls/${PR}" \
  --jq '{number, base: {ref: .base.ref, sha: .base.sha}, head: {ref: .head.ref, sha: .head.sha}, mergeable_state, changed_files, commits, additions, deletions}'

echo '=== Pull request file list ==='
gh api --paginate "repos/${REPO}/pulls/${PR}/files?per_page=100" \
  --jq '.[] | [.status, .filename, .additions, .deletions] | `@tsv`'

echo '=== Fetch target branch for comparison ==='
git fetch --no-tags --depth=1 origin \
  "feature-62/bucket-album-approach-tags-owners-root:refs/remotes/origin/pr-base-4711" || true

if git rev-parse --verify -q origin/pr-base-4711 >/dev/null; then
  BASE="$(git merge-base HEAD origin/pr-base-4711)"
  echo "BASE=$BASE"
  echo '=== Full PR stat ==='
  git diff --stat "$BASE"...HEAD
  echo '=== Documentation conflict-marker diff ==='
  git diff --unified=15 "$BASE"...HEAD -- \
    docs/specs/4-architecture/open-questions.md \
    docs/specs/_current-session.md
fi

echo '=== Current conflict-marker provenance ==='
git log -S'<<<<<<< HEAD' --format='%H %s' --all -- \
  docs/specs/4-architecture/open-questions.md \
  docs/specs/_current-session.md || true

echo '=== Selected sources ==='
for f in \
  resources/js/stores/AlbumState.ts \
  resources/js/stores/AlbumsState.ts \
  resources/js/composables/album/dragAndSelect.ts \
  resources/js/v8/composables/album/virtualAlbumRows.ts \
  resources/js/v8/utils/albumBucketBoundaries.ts \
  resources/js/v8/utils/adaptAlbumChildTile.ts \
  resources/js/v8/utils/adaptCategoryTile.ts \
  resources/js/services/album-category-v3-service.ts \
  resources/js/services/album-children-v3-service.ts
do
  echo "--- $f ---"
  cat -n "$f"
done

Length of output: 175400


🏁 Script executed:

#!/bin/bash
set -euo pipefail

BASE="52d8f208083f1cf421120b05f7b80d890ee3ad38"

echo '=== selectableAlbums definitions and consumers ==='
rg -n -C 5 '\bselectableAlbums\b' resources/js

echo '=== v3 shared-root state consumers ==='
rg -n -C 4 '\bsharedAlbumsV3\b|\bsharedAlbums\b' \
  resources/js/v8 resources/js/composables resources/js/stores

echo '=== Complete diffs for affected stores and drag selection ==='
git diff --unified=25 "$BASE"...HEAD -- \
  resources/js/stores/AlbumsState.ts \
  resources/js/stores/AlbumState.ts \
  resources/js/composables/album/dragAndSelect.ts \
  resources/js/services/album-service.ts

echo '=== Virtual component sources ==='
for f in \
  resources/js/v8/components/gallery/albumModule/Virtualized/AlbumThumbGridVirtual.vue \
  resources/js/v8/components/gallery/albumModule/Virtualized/AlbumRootGridVirtual.vue \
  resources/js/v8/components/gallery/albumModule/Virtualized/AlbumListViewVirtual.vue \
  resources/js/v8/components/gallery/albumModule/Virtualized/AlbumRootListViewVirtual.vue \
  resources/js/v8/components/gallery/albumModule/Virtualized/AlbumRootPanelVirtual.vue \
  resources/js/v8/components/gallery/albumModule/Virtualized/AlbumThumbPanelVirtual.vue \
  resources/js/v8/views/gallery-panels/Albums.vue \
  resources/js/v8/components/gallery/albumModule/AlbumPanel.vue
do
  echo "--- $f ---"
  cat -n "$f"
done

echo '=== Tests for new helpers or V3 stores ==='
fd -t f . resources/js | rg '(test|spec)\.(ts|tsx|vue)$' | head -200
rg -n -C 2 'computeBucketBoundaries|buildVirtualAlbumRows|adaptAlbumChildTile|loadAlbumsV3|loadRootAlbumsV3' \
  . -g '*.{test,spec}.{ts,tsx,js}' -g '!node_modules/**' || true

Length of output: 169475


@ildyria Review complete. I found three issues.

  1. Blocking — resolve the merge-conflict markers.
    docs/specs/4-architecture/open-questions.md contains <<<<<<<, =======, and >>>>>>> markers at Lines 171, 213, 573, 578, 719, and 721. The PR diff introduces these markers, and GitHub reports the PR as blocked. Resolve the conflict and retain only the intended question-detail sections.

  2. High — include sharedAlbumsV3 in selectableAlbums.
    resources/js/stores/AlbumsState.ts Line 63 builds selectableAlbums from pinnedAlbums, albums, and legacy sharedAlbums, but excludes sharedAlbumsV3. The Struct-of-Array root view renders shared albums from sharedAlbumsV3. As a result, drag selection generates shared-album IDs in resources/js/composables/album/dragAndSelect.ts, but Line 322 rejects them during reduceIntersection(). The same omission also breaks context-menu data, keyboard selection, range selection, and bulk actions for shared root albums. Add the v3 shared collection to the selectable source, with the existing ID de-duplication.

  3. High — apply separate_shared_only filtering to the v3 shared panel.
    resources/js/v8/views/gallery-panels/Albums.vue Lines 443-447 use !album.is_public only to decide if the shared panel exists. The rendered AlbumRootPanelVirtual at Lines 76-82 and Lines 130-136 still receives all sharedAlbumsV3 tiles. Therefore, when one directly shared album exists, the Struct-of-Array path also displays public albums. The legacy path filters these albums in displaySharedAlbums. Filter the v3 shared tile set before rendering, or pass a visibility-filtered collection to the virtual panel.

⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant