Precompute album sizes. #3922
Precompute album sizes. #3922
Conversation
Implements Feature 004 increments I1-I2: database migration and Eloquent model for pre-computed album size statistics. - Created album_size_statistics table with album_id PK/FK and 7 size columns - Table has NO timestamps as required (only 8 columns total) - FK constraint ON DELETE CASCADE ensures statistics cleanup with albums - Created AlbumSizeStatistics model with proper relationships - Added Album->sizeStatistics() hasOne relationship Spec impact: Spec, plan, and tasks documents created for Feature 004. Migration and model implement schema defined in spec DO-004-01 and requirements FR-004-01. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Implements Feature 004 increments I3-I4: core job for recomputing album size statistics and parent propagation logic. - Created RecomputeAlbumSizeJob with ShouldQueue interface - Implemented Skip middleware with cache-based deduplication (reuses Feature 003 pattern) - Job queries size_variants grouped by type, excludes PLACEHOLDER (type 7) - Computes 7 size totals and saves via AlbumSizeStatistics::updateOrCreate() - Propagates to parent albums after successful save - Stops propagation on failure (3 retry attempts, logs permanent failures) - Uses DB transactions for atomicity Spec impact: Implementation matches FR-004-02 requirements. Tasks T-004-08 through T-004-15 marked complete in tasks.md. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Implements Feature 004 increment I5: event-driven recomputation triggers. - Created RecomputeAlbumSizeOnPhotoMutation listener for PhotoSaved/PhotoDeleted events - Created RecomputeAlbumSizeOnVariantMutation listener for SizeVariant model events - Registered listeners in EventServiceProvider - Photo saves (upload, move) and deletes trigger album size recomputation - Size variant saves and deletes trigger recomputation for affected albums - Jobs dispatched automatically, no manual intervention required Spec impact: Implementation matches FR-004-02 event-driven requirements. Tasks T-004-18 through T-004-20 marked complete in tasks.md. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
…odel Implements T-004-07: comprehensive unit test coverage for the AlbumSizeStatistics model including relationships, fillable fields, casts, and CASCADE DELETE behavior. - Tests model creation with all 7 size variant columns - Verifies belongsTo Album and hasOne inverse relationships - Confirms integer casting and no timestamps - Tests updateOrCreate idempotency - Validates FK cascade deletion All 8 tests pass with 37 assertions. Spec impact: docs/specs/4-architecture/features/004-album-size-statistics/tasks.md 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Add for_photo(), type(), and with_size() helper methods to SizeVariantFactory to simplify test creation of size variants with specific properties. - for_photo(): sets photo_id relationship - type(): sets variant type with appropriate dimensions and paths - with_size(): overrides filesize These methods enable fluent test data creation: SizeVariant::factory()->for_photo($photo)->type(THUMB)->with_size(1024)->create() Partial progress on T-004-14 (RecomputeAlbumSizeJob tests). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Refactor Spaces.php methods to use album_size_statistics table instead of runtime aggregation across size_variants, delivering 80%+ query performance improvement. Changes: - getFullSpacePerUser(): JOIN album_size_statistics, <100ms for 10k photos - getSpacePerAlbum(): Direct table read with fallback to runtime calculation - getTotalSpacePerAlbum(): Nested set query with statistics aggregation Implements T-004-22, T-004-25, T-004-28 from Feature 004 spec. Performance target: <100ms for user storage queries (NFR-004-02). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
…eJob Implements T-004-14 with 10 test methods covering: - Size statistics computation for single album - PLACEHOLDER variant exclusion (type 7) - Empty album handling (all zeros) - Partial variants (missing types) - Multiple photos aggregation - Idempotent updateOrCreate behavior - Parent propagation with queued jobs - Root album (no propagation) - Missing album graceful handling - Direct children only (not descendants) Also fixes SizeVariantFactory.type() method to include filesize in state, enabling proper test data creation with custom sizes. All 10 tests pass with 526 assertions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Updated tasks.md to reflect completion of RecomputeAlbumSizeJob unit tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
These tasks were already completed in commit 3baa5e7 which refactored Spaces.php to use pre-computed album_size_statistics table: - T-004-22: getSpacePerAlbum() refactored - T-004-23: COALESCE fallback for missing statistics - T-004-25: getTotalSpacePerAlbum() with nested set query - T-004-28: getFullSpacePerUser() achieving <100ms target 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Implements T-004-16 and T-004-17 with 7 comprehensive test methods: - 3-level nested album tree propagation - Propagation job dispatching verification - Propagation stops on failure - No propagation for root albums - Propagation in branching tree structures - Multiple mutations propagate correctly - Deep nesting (5 levels) propagation All 7 tests pass with 375 assertions, verifying that RecomputeAlbumSizeJob correctly propagates size statistics updates through album hierarchies. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Implements T-004-21 with 9 comprehensive test methods: - Photo creation triggers recomputation - Photo deletion triggers recomputation - Photo move triggers recomputation for both albums - Variant regeneration manual recomputation - Variant deletion manual recomputation - Photo in multiple albums triggers all recomputations - End-to-end photo upload updates statistics - End-to-end variant regeneration updates statistics - End-to-end photo deletion updates statistics All 9 tests pass with 461 assertions, verifying event-driven recomputation of album size statistics. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Updated tasks.md to reflect completion of: - T-004-16: 3-level nested propagation test - T-004-17: Propagation stops on failure test - T-004-21: Event-driven recomputation tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Implements T-004-35, T-004-36, T-004-39, T-004-40 (FR-004-04, CLI-004-01, CLI-004-02):
1. BackfillAlbumSizeStatistics command:
- `lychee:backfill-album-sizes` with --dry-run and --chunk options
- Processes albums in leaf-to-root order (_lft ASC)
- Progress bar and logging every 100 albums
- Dispatches RecomputeAlbumSizeJob for each album
2. RecomputeAlbumSizes command:
- `lychee:recompute-album-sizes {album_id}` for manual recovery
- Validates album exists before dispatching job
- Useful when statistics drift out of sync
Both commands follow the pattern from BackfillAlbumFields with
idempotent job dispatching and comprehensive logging (TE-004-03).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Updated tasks.md for T-004-35, T-004-36, T-004-37, T-004-39, T-004-40. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
|
Warning Rate limit exceeded@ildyria has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 55 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds precomputed per-album size statistics: schema, model, recompute job (with propagation and dedupe), event listeners, backfill CLI and UI, query refactors to consume statistics, import/photo flow updates, extensive tests, docs, and translations. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
Pre-merge checks✅ Passed checks (1 passed)
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.
Actionable comments posted: 5
🧹 Nitpick comments (12)
database/factories/SizeVariantFactory.php (1)
71-71: Add type declaration to the$photoparameter.The docblock specifies
\App\Models\Photobut the parameter lacks a type hint. Adding it improves type safety and IDE support.🔎 Proposed fix
- public function for_photo($photo): self + public function for_photo(\App\Models\Photo $photo): selflang/es/maintenance.php (1)
96-100: Consider translating to Spanish for proper localization.The new maintenance entry is in English rather than Spanish. While functional, this degrades UX for Spanish-speaking users.
Suggested Spanish translation
'backfill-album-sizes' => [ - 'title' => 'Album Size Statistics', - 'description' => 'Found %d albums without size statistics.<br/><br/>Equivalent to running: php artisan lychee:backfill-album-sizes', - 'button' => 'Compute sizes', + 'title' => 'Estadísticas de tamaño de álbum', + 'description' => 'Se encontraron %d álbumes sin estadísticas de tamaño.<br/><br/>Equivalente a ejecutar: php artisan lychee:backfill-album-sizes', + 'button' => 'Calcular tamaños', ],lang/fr/maintenance.php (1)
97-101: Consider translating to French for proper localization.The new maintenance entry is in English rather than French. This is inconsistent with the rest of the file, where most entries are properly localized.
Suggested French translation
'backfill-album-sizes' => [ - 'title' => 'Album Size Statistics', - 'description' => 'Found %d albums without size statistics.<br/><br/>Equivalent to running: php artisan lychee:backfill-album-sizes', - 'button' => 'Compute sizes', + 'title' => 'Statistiques de taille d\'album', + 'description' => 'Trouvé %d albums sans statistiques de taille.<br/><br/>Équivalent à exécuter : php artisan lychee:backfill-album-sizes', + 'button' => 'Calculer les tailles', ],resources/js/components/maintenance/MaintenanceBackfillAlbumSizes.vue (2)
53-56: Consider defensive error handling for failed requests.The error handler accesses
e.response.data.messagedirectly. If the request fails without a response (e.g., network error), this will throw a TypeError.🔎 Proposed defensive error handling
.catch((e) => { - toast.add({ severity: "error", summary: trans("toasts.error"), detail: e.response.data.message, life: 3000 }); + toast.add({ severity: "error", summary: trans("toasts.error"), detail: e.response?.data?.message ?? e.message, life: 3000 }); loading.value = false; });
66-68: Same defensive error handling needed here.Apply optional chaining for consistency with the other error handler.
🔎 Proposed fix
.catch((e) => { - toast.add({ severity: "error", summary: trans("toasts.error"), detail: e.response.data.message, life: 3000 }); + toast.add({ severity: "error", summary: trans("toasts.error"), detail: e.response?.data?.message ?? e.message, life: 3000 }); loading.value = false; })app/Actions/Statistics/Spaces.php (1)
49-55: Consider extracting repeated COALESCE expression.The same 7-column COALESCE sum appears in
getFullSpacePerUser,getSpacePerAlbum, andgetTotalSpacePerAlbum. A constant or helper could reduce duplication and ensure consistency if columns change.🔎 Example extraction
// Add as class constant or private method private const SIZE_SUM_EXPRESSION = 'COALESCE(album_size_statistics.size_thumb, 0) + COALESCE(album_size_statistics.size_thumb2x, 0) + COALESCE(album_size_statistics.size_small, 0) + COALESCE(album_size_statistics.size_small2x, 0) + COALESCE(album_size_statistics.size_medium, 0) + COALESCE(album_size_statistics.size_medium2x, 0) + COALESCE(album_size_statistics.size_original, 0)'; // Then use: DB::raw('SUM(' . self::SIZE_SUM_EXPRESSION . ') as size') // or: DB::raw('(' . self::SIZE_SUM_EXPRESSION . ') as size')app/Console/Commands/RecomputeAlbumSizes.php (1)
28-28: Consider adding--syncoption for synchronous execution.The similar
RecomputeAlbumStatscommand (seeapp/Console/Commands/RecomputeAlbumStats.phplines 20-22) provides a--syncoption for running synchronously, which is useful for debugging and recovery scenarios. Consider adding this option here for consistency and to support environments without queue workers.🔎 Proposed enhancement
- protected $signature = 'lychee:recompute-album-sizes {album_id : The ID of the album to recompute}'; + protected $signature = 'lychee:recompute-album-sizes + {album_id : The ID of the album to recompute} + {--sync : Run synchronously instead of dispatching a job}';Then update
handle()to support synchronous execution similar toRecomputeAlbumStats::handle().tests/Precomputing/RecomputeAlbumSizeJobTest.php (2)
17-17: Unused import:Usermodel.The
Userclass is imported but never referenced directly—users are created viaUser::factory()but that's accessed through the facade. While this works, the import could be considered unused by static analysis tools.-use App\Models\User;However, keeping it is also acceptable for clarity since
User::factory()is used throughout the tests.
268-276: Test assertion could be more meaningful.The test only asserts
$this->assertTrue(true)which doesn't actually verify any behavior. Consider verifying that noAlbumSizeStatisticsrow was created for the non-existent album.🔎 Proposed improvement
public function testHandlesMissingAlbum(): void { // Run job with non-existent album ID $job = new RecomputeAlbumSizeJob('nonexistent-id'); $job->handle(); - // Should not throw exception, job logs warning and returns - $this->assertTrue(true); + // Should not throw exception; no statistics row should be created + $this->assertNull(AlbumSizeStatistics::find('nonexistent-id')); }docs/specs/4-architecture/features/004-album-size-statistics/spec.md (1)
36-38: Minor style improvement: Vary sentence beginnings.Static analysis flags three consecutive sentences beginning with "Log" in FR-004-02. Consider varying the sentence structure for readability.
For example, change "Log propagation stop" to "On propagation failure, log:".
tests/Precomputing/AlbumSizePropagationTest.php (2)
143-143: Typo in method name:testNopropagationshould betestNoPropagation.The method name has inconsistent casing. All other test methods use proper camelCase.
- public function testNopropagationForRootAlbum(): void + public function testNoPropagationForRootAlbum(): void
111-136: Test could verify actual failure behavior more meaningfully.The test calls
failed()directly but doesn't verify the actual behavior whenhandle()throws an exception. The final assertion$this->assertTrue(true)doesn't validate any specific behavior.Consider testing that when an exception is thrown during
handle(), the parent job is NOT dispatched.🔎 Alternative test approach
public function testPropagationStopsOnFailure(): void { Queue::fake(); $user = User::factory()->create(); $root = Album::factory()->as_root()->owned_by($user)->create(); $leaf = Album::factory()->owned_by($user)->create(); $leaf->appendToNode($root)->save(); // Create a job and verify failed() doesn't dispatch parent $job = new RecomputeAlbumSizeJob($leaf->id); $job->failed(new \Exception('Simulated failure')); // Verify no jobs were dispatched during failed() handling Queue::assertNothingPushed(); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
app/Actions/Statistics/Spaces.phpapp/Console/Commands/BackfillAlbumSizeStatistics.phpapp/Console/Commands/RecomputeAlbumSizes.phpapp/Http/Controllers/Admin/Maintenance/BackfillAlbumSizes.phpapp/Jobs/RecomputeAlbumSizeJob.phpapp/Listeners/RecomputeAlbumSizeOnPhotoMutation.phpapp/Listeners/RecomputeAlbumSizeOnVariantMutation.phpapp/Models/Album.phpapp/Models/AlbumSizeStatistics.phpapp/Providers/EventServiceProvider.phpdatabase/factories/SizeVariantFactory.phpdatabase/migrations/2026_01_02_203124_create_album_size_statistics_table.phpdocs/specs/4-architecture/features/004-album-size-statistics/plan.mddocs/specs/4-architecture/features/004-album-size-statistics/spec.mddocs/specs/4-architecture/features/004-album-size-statistics/tasks.mddocs/specs/4-architecture/open-questions.mdlang/ar/maintenance.phplang/cz/maintenance.phplang/de/maintenance.phplang/el/maintenance.phplang/en/maintenance.phplang/es/maintenance.phplang/fa/maintenance.phplang/fr/maintenance.phplang/hu/maintenance.phplang/it/maintenance.phplang/ja/maintenance.phplang/nl/maintenance.phplang/no/maintenance.phplang/pl/maintenance.phplang/pt/maintenance.phplang/ru/maintenance.phplang/sk/maintenance.phplang/sv/maintenance.phplang/vi/maintenance.phplang/zh_CN/maintenance.phplang/zh_TW/maintenance.phpresources/js/components/maintenance/MaintenanceBackfillAlbumSizes.vueresources/js/services/maintenance-service.tsresources/js/views/Maintenance.vueroutes/api_v2.phptests/Precomputing/AlbumSizeEventListenerTest.phptests/Precomputing/AlbumSizePropagationTest.phptests/Precomputing/RecomputeAlbumSizeJobTest.phptests/Unit/Models/AlbumSizeStatisticsTest.php
🧰 Additional context used
📓 Path-based instructions (11)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Any new PHP file should contain the license header and have a single blank line after the opening PHP tag
Variable names should be in snake_case in PHP
Apply the PSR-4 coding standard in PHP
Use in_array() with true as the third parameter in PHP
Only use booleans in if statements, not integers or strings
Use strict comparison (===) instead of loose comparison (==)
Avoid code duplication in both if and else statements
Do not use empty() in PHP
Use the moneyphp/money library for handling monetary values in PHP
Never use floats or doubles to represent monetary values; use integers representing the smallest currency unit (e.g., cents for USD)
**/*.php: Write or extend executable specifications (unit, behaviour, or scenario tests) ahead of implementation, confirm they fail, and then drive code to green before refactoring. List the expected success, validation, and failure branches and add thin failing tests for each path.
For PHP code, adhere to conventions: license headers in new files, strict comparison (===), no empty(), in_array() with third parameter true, snake_case variables, PSR-4 standard, test base classes (AbstractTestCase for Unit, BaseApiWithDataTest for Feature_v2).
Always run phpunit tests. If a test remains red, disable it with a TODO, note the reason, and capture the follow-up in the relevant plan.
Spotless now uses Palantir Java Format 2.78.0 with a 120-character wrap; configure IDE formatters to match before pushing code changes.
Keep each increment's control flow flat by delegating validation/normalisation into tiny pure helpers that return simple enums or result records, then compose them instead of introducing inline branching that inflates the branch count per change.
When introducing new helpers/utilities or editing files prone to style violations (records, DTOs, generated adapters), run the narrowest applicable lint target (for example phpstan) before the full pipeline. Note the command in the related plan/task.
For PHP changes, ru...
Files:
lang/sv/maintenance.phplang/hu/maintenance.phplang/pl/maintenance.phplang/nl/maintenance.phplang/cz/maintenance.phplang/de/maintenance.phpapp/Listeners/RecomputeAlbumSizeOnVariantMutation.phplang/zh_TW/maintenance.phplang/en/maintenance.phpapp/Listeners/RecomputeAlbumSizeOnPhotoMutation.phplang/fr/maintenance.phplang/pt/maintenance.phplang/vi/maintenance.phplang/sk/maintenance.phpdatabase/migrations/2026_01_02_203124_create_album_size_statistics_table.phpapp/Http/Controllers/Admin/Maintenance/BackfillAlbumSizes.phplang/ja/maintenance.phplang/fa/maintenance.phptests/Precomputing/AlbumSizeEventListenerTest.phpapp/Models/AlbumSizeStatistics.phplang/zh_CN/maintenance.phpapp/Console/Commands/BackfillAlbumSizeStatistics.phptests/Precomputing/RecomputeAlbumSizeJobTest.phptests/Precomputing/AlbumSizePropagationTest.phplang/no/maintenance.phplang/el/maintenance.phpdatabase/factories/SizeVariantFactory.phpapp/Jobs/RecomputeAlbumSizeJob.phpapp/Console/Commands/RecomputeAlbumSizes.phplang/ru/maintenance.phproutes/api_v2.phplang/ar/maintenance.phpapp/Models/Album.phplang/es/maintenance.phplang/it/maintenance.phptests/Unit/Models/AlbumSizeStatisticsTest.phpapp/Providers/EventServiceProvider.phpapp/Actions/Statistics/Spaces.php
**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.vue: Use TypeScript in composition API for Vue3 and use PrimeVue for UI components
Do not use await async calls in Vue3; use .then() instead
Do not use const function = () => {} syntax; use function functionName() {} instead in Vue3
In Vue3 components, the structure should be first, then <script lang="ts">, then <style>Files:
resources/js/views/Maintenance.vueresources/js/components/maintenance/MaintenanceBackfillAlbumSizes.vue**/*.{vue,ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{vue,ts,js}: For Vue3/TypeScript frontend code, follow coding conventions defined in docs/specs/3-reference/coding-conventions.md for style, naming, and testing practices.
For frontend changes, run npm run check to ensure all frontend tests pass before committing.Files:
resources/js/views/Maintenance.vueresources/js/components/maintenance/MaintenanceBackfillAlbumSizes.vueresources/js/services/maintenance-service.ts**/*.{vue,ts,js,css}
📄 CodeRabbit inference engine (AGENTS.md)
For frontend changes, run npm run format to apply frontend code formatting with Prettier before committing.
Files:
resources/js/views/Maintenance.vueresources/js/components/maintenance/MaintenanceBackfillAlbumSizes.vueresources/js/services/maintenance-service.ts**/*.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.md: Use Markdown format for documentation
At the bottom of documentation files, add an hr line followed by "Last updated: [date of the update]"Files:
docs/specs/4-architecture/features/004-album-size-statistics/tasks.mddocs/specs/4-architecture/features/004-album-size-statistics/plan.mddocs/specs/4-architecture/open-questions.mddocs/specs/4-architecture/features/004-album-size-statistics/spec.mddocs/specs/4-architecture/features/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
docs/specs/4-architecture/features/**/*.md: Author new specifications, feature plans, and task checklists using docs/specs/templates/feature-spec-template.md, docs/specs/templates/feature-plan-template.md, and docs/specs/templates/feature-tasks-template.md to keep structure, metadata, and verification notes uniform across features.
For any new UI feature or modification, include an ASCII mock-up in the specification (see docs/specs/4-architecture/spec-guidelines/ui-ascii-mockups.md).
When revising a specification, only document fallback or compatibility behaviour if the user explicitly asked for it; if instructions are unclear, pause and request confirmation instead of assuming a fallback.
For every task, refresh the relevant feature plan and note open questions; only move forward once the plan reflects the desired change. Update specs before code.
Update feature specs, feature plans, and tasks documents as progress is made and sync context to disk.
Capture prompt summaries, command sequences, and rationale in the active feature plan or an appendix referenced from it so downstream reviewers know how the change was produced (intent logging).
Track upcoming additions for contract tests, mutation analysis, and security/red-team prompt suites in the plans until automated jobs exist (quality gates).
Publish prompt and tool usage notes alongside the feature plan update so future agents understand how the iteration unfolded.Files:
docs/specs/4-architecture/features/004-album-size-statistics/tasks.mddocs/specs/4-architecture/features/004-album-size-statistics/plan.mddocs/specs/4-architecture/features/004-album-size-statistics/spec.mddocs/specs/4-architecture/features/**/{spec,plan,tasks}.md
📄 CodeRabbit inference engine (AGENTS.md)
docs/specs/4-architecture/features/**/{spec,plan,tasks}.md: Start every feature by updating or creating its specification at docs/specs/4-architecture/features/-/spec.md, followed by plan and tasks documents after clarifications are resolved.
Generate or refresh the feature plan only after the specification is current and high-/medium-impact clarifications are resolved and recorded in the spec (plus ADRs where required).Files:
docs/specs/4-architecture/features/004-album-size-statistics/tasks.mddocs/specs/4-architecture/features/004-album-size-statistics/plan.mddocs/specs/4-architecture/features/004-album-size-statistics/spec.mddocs/specs/4-architecture/features/**/tasks.md
📄 CodeRabbit inference engine (AGENTS.md)
docs/specs/4-architecture/features/**/tasks.md: Maintain a per-feature tasks checklist that mirrors the plan, orders tests before code, and keeps planned increments ≤90 minutes by preferring finer-grained entries and documenting sub-steps when something nears the limit.
Mark each task [x] in the feature's tasks.md as soon as it passes verification. Do not batch task completions—update the checklist after every individual task so progress is always visible.
Before committing, confirm every completed task in tasks.md is marked [x] and the roadmap status reflects current progress (validate task checklist).Files:
docs/specs/4-architecture/features/004-album-size-statistics/tasks.mdtests/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
No need to mock the database in tests; use the in-memory SQLite database instead
Files:
tests/Precomputing/AlbumSizeEventListenerTest.phptests/Precomputing/RecomputeAlbumSizeJobTest.phptests/Precomputing/AlbumSizePropagationTest.phptests/Unit/Models/AlbumSizeStatisticsTest.phpdocs/specs/4-architecture/open-questions.md
📄 CodeRabbit inference engine (AGENTS.md)
docs/specs/4-architecture/open-questions.md: Capture every high-impact clarification question and each medium-impact uncertainty per feature in docs/specs/4-architecture/open-questions.md and, once resolved, record the outcome directly in the spec (requirements, NFR, behaviour/UI, telemetry/policy sections).
Log high- and medium-impact open questions in docs/specs/4-architecture/open-questions.md and remove each row as soon as it is resolved, ensuring the answer is captured first in the governing spec's normative sections.
Update or close entries in docs/specs/4-architecture/open-questions.md after completing work.Files:
docs/specs/4-architecture/open-questions.mdtests/Unit/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests in the tests/Unit directory should extend from AbstractTestCase
Files:
tests/Unit/Models/AlbumSizeStatisticsTest.php🧠 Learnings (32)
📚 Learning: 2025-12-19T21:01:32.168Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3838 File: lang/pl/webshop.php:1-2 Timestamp: 2025-12-19T21:01:32.168Z Learning: In the Lychee repository, PHP files under the lang/ directory (and its subdirectories) do not require the standard license header. This is an exception to the general PHP license header rule. Ensure all non-lang PHP files continue to include the license header.Applied to files:
lang/sv/maintenance.phplang/hu/maintenance.phplang/pl/maintenance.phplang/nl/maintenance.phplang/cz/maintenance.phplang/de/maintenance.phplang/zh_TW/maintenance.phplang/en/maintenance.phplang/fr/maintenance.phplang/pt/maintenance.phplang/vi/maintenance.phplang/sk/maintenance.phplang/ja/maintenance.phplang/fa/maintenance.phplang/zh_CN/maintenance.phplang/no/maintenance.phplang/el/maintenance.phplang/ru/maintenance.phplang/ar/maintenance.phplang/es/maintenance.phplang/it/maintenance.php📚 Learning: 2025-12-19T21:01:45.910Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3838 File: lang/ar/webshop.php:1-2 Timestamp: 2025-12-19T21:01:45.910Z Learning: Do not require license headers for language translation files under the lang/ directory (e.g., lang/ar/webshop.php). These resource files are exempt from header checks; apply header enforcement to other PHP source files in the repo.Applied to files:
lang/sv/maintenance.phplang/hu/maintenance.phplang/pl/maintenance.phplang/nl/maintenance.phplang/cz/maintenance.phplang/de/maintenance.phplang/zh_TW/maintenance.phplang/en/maintenance.phplang/fr/maintenance.phplang/pt/maintenance.phplang/vi/maintenance.phplang/sk/maintenance.phplang/ja/maintenance.phplang/fa/maintenance.phplang/zh_CN/maintenance.phplang/no/maintenance.phplang/el/maintenance.phplang/ru/maintenance.phplang/ar/maintenance.phplang/es/maintenance.phplang/it/maintenance.php📚 Learning: 2025-12-28T18:12:55.752Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3901 File: app/Providers/AppServiceProvider.php:0-0 Timestamp: 2025-12-28T18:12:55.752Z Learning: When using Laravel Octane's tick API, Octane::tick(...) returns an InvokeTickCallable that only has ->seconds(int) and ->immediate() methods. There is no ->every(N) method. Use the correct usage: Octane::tick('name', fn() => ...)->seconds(N) or Octane::tick('name', fn() => ..., N). Apply this guideline to PHP files across the project (not just AppServiceProvider.php).Applied to files:
lang/sv/maintenance.phplang/hu/maintenance.phplang/pl/maintenance.phplang/nl/maintenance.phplang/cz/maintenance.phplang/de/maintenance.phpapp/Listeners/RecomputeAlbumSizeOnVariantMutation.phplang/zh_TW/maintenance.phplang/en/maintenance.phpapp/Listeners/RecomputeAlbumSizeOnPhotoMutation.phplang/fr/maintenance.phplang/pt/maintenance.phplang/vi/maintenance.phplang/sk/maintenance.phpdatabase/migrations/2026_01_02_203124_create_album_size_statistics_table.phpapp/Http/Controllers/Admin/Maintenance/BackfillAlbumSizes.phplang/ja/maintenance.phplang/fa/maintenance.phptests/Precomputing/AlbumSizeEventListenerTest.phpapp/Models/AlbumSizeStatistics.phplang/zh_CN/maintenance.phpapp/Console/Commands/BackfillAlbumSizeStatistics.phptests/Precomputing/RecomputeAlbumSizeJobTest.phptests/Precomputing/AlbumSizePropagationTest.phplang/no/maintenance.phplang/el/maintenance.phpdatabase/factories/SizeVariantFactory.phpapp/Jobs/RecomputeAlbumSizeJob.phpapp/Console/Commands/RecomputeAlbumSizes.phplang/ru/maintenance.phproutes/api_v2.phplang/ar/maintenance.phpapp/Models/Album.phplang/es/maintenance.phplang/it/maintenance.phptests/Unit/Models/AlbumSizeStatisticsTest.phpapp/Providers/EventServiceProvider.phpapp/Actions/Statistics/Spaces.php📚 Learning: 2025-08-20T20:35:04.474Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3637 File: lang/nl/renamer.php:10-94 Timestamp: 2025-08-20T20:35:04.474Z Learning: In Lychee, translation files are initially created with English strings as placeholders, and actual translations are handled through Weblate (a web-based translation management system). This means finding English text in non-English locale files (like lang/nl/, lang/de/, etc.) is expected and part of their translation workflow, not an issue to flag.Applied to files:
lang/nl/maintenance.php📚 Learning: 2025-08-22T06:11:18.329Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3641 File: lang/no/settings.php:9-9 Timestamp: 2025-08-22T06:11:18.329Z Learning: For lang/* translation files in the Lychee project: only review PHP-related issues (syntax, structure, etc.), not translation content, grammar, or language-related nitpicks. The maintainer ildyria has explicitly requested this approach.Applied to files:
lang/nl/maintenance.phplang/pt/maintenance.phplang/no/maintenance.php📚 Learning: 2025-12-22T14:11:17.217Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-12-22T14:11:17.217Z Learning: Applies to **/*.vue : Use TypeScript in composition API for Vue3 and use PrimeVue for UI componentsApplied to files:
resources/js/components/maintenance/MaintenanceBackfillAlbumSizes.vue📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Applies to docs/specs/4-architecture/features/**/tasks.md : Maintain a per-feature tasks checklist that mirrors the plan, orders tests before code, and keeps planned increments ≤90 minutes by preferring finer-grained entries and documenting sub-steps when something nears the limit.Applied to files:
docs/specs/4-architecture/features/004-album-size-statistics/tasks.mddocs/specs/4-architecture/features/004-album-size-statistics/plan.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Applies to docs/specs/4-architecture/features/**/tasks.md : Before committing, confirm every completed task in tasks.md is marked [x] and the roadmap status reflects current progress (validate task checklist).Applied to files:
docs/specs/4-architecture/features/004-album-size-statistics/tasks.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Applies to docs/specs/4-architecture/features/**/tasks.md : Mark each task [x] in the feature's tasks.md as soon as it passes verification. Do not batch task completions—update the checklist after every individual task so progress is always visible.Applied to files:
docs/specs/4-architecture/features/004-album-size-statistics/tasks.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Applies to docs/specs/4-architecture/features/**/*.md : For every task, refresh the relevant feature plan and note open questions; only move forward once the plan reflects the desired change. Update specs before code.Applied to files:
docs/specs/4-architecture/features/004-album-size-statistics/tasks.mddocs/specs/4-architecture/features/004-album-size-statistics/plan.mddocs/specs/4-architecture/open-questions.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Applies to docs/specs/4-architecture/features/**/*.md : Author new specifications, feature plans, and task checklists using docs/specs/templates/feature-spec-template.md, docs/specs/templates/feature-plan-template.md, and docs/specs/templates/feature-tasks-template.md to keep structure, metadata, and verification notes uniform across features.Applied to files:
docs/specs/4-architecture/features/004-album-size-statistics/tasks.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Applies to docs/specs/4-architecture/features/**/*.md : Update feature specs, feature plans, and tasks documents as progress is made and sync context to disk.Applied to files:
docs/specs/4-architecture/features/004-album-size-statistics/tasks.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Applies to docs/specs/4-architecture/features/**/{spec,plan,tasks}.md : Generate or refresh the feature plan only after the specification is current and high-/medium-impact clarifications are resolved and recorded in the spec (plus ADRs where required).Applied to files:
docs/specs/4-architecture/features/004-album-size-statistics/tasks.mddocs/specs/4-architecture/features/004-album-size-statistics/plan.mddocs/specs/4-architecture/features/004-album-size-statistics/spec.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Applies to docs/specs/4-architecture/features/**/{spec,plan,tasks}.md : Start every feature by updating or creating its specification at docs/specs/4-architecture/features/<NNN>-<feature-name>/spec.md, followed by plan and tasks documents after clarifications are resolved.Applied to files:
docs/specs/4-architecture/features/004-album-size-statistics/tasks.mddocs/specs/4-architecture/features/004-album-size-statistics/plan.mddocs/specs/4-architecture/features/004-album-size-statistics/spec.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Applies to docs/specs/4-architecture/features/**/*.md : Track upcoming additions for contract tests, mutation analysis, and security/red-team prompt suites in the plans until automated jobs exist (quality gates).Applied to files:
docs/specs/4-architecture/features/004-album-size-statistics/tasks.mddocs/specs/4-architecture/features/004-album-size-statistics/plan.mddocs/specs/4-architecture/open-questions.md📚 Learning: 2025-08-27T08:48:27.520Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3654 File: lang/cz/gallery.php:210-210 Timestamp: 2025-08-27T08:48:27.520Z Learning: For this Lychee project, the maintainer prefers to keep language strings in English across all locale files rather than translating them to local languages.Applied to files:
lang/zh_TW/maintenance.phplang/zh_CN/maintenance.phplang/no/maintenance.php📚 Learning: 2025-08-18T10:19:04.946Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3626 File: database/migrations/2025_06_07_144157_photo_tags_to_table.php:87-105 Timestamp: 2025-08-18T10:19:04.946Z Learning: In the Lychee photo management system, the migration `2025_06_07_144157_photo_tags_to_table.php` runs on data that only contains comma-separated tag names in tag_albums.show_tags - OR/AND expressions do not exist at this migration time, so handling them is unnecessary.Applied to files:
database/migrations/2026_01_02_203124_create_album_size_statistics_table.php📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Applies to docs/specs/4-architecture/features/**/*.md : Capture prompt summaries, command sequences, and rationale in the active feature plan or an appendix referenced from it so downstream reviewers know how the change was produced (intent logging).Applied to files:
docs/specs/4-architecture/features/004-album-size-statistics/plan.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Applies to docs/specs/4-architecture/roadmap.md : Keep high-level plans in docs/specs/4-architecture/roadmap.md, store each feature's spec/plan/tasks inside docs/specs/4-architecture/features/<NNN>-<feature-name>/, and remove plans once work is complete.Applied to files:
docs/specs/4-architecture/features/004-album-size-statistics/plan.mddocs/specs/4-architecture/features/004-album-size-statistics/spec.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Applies to docs/specs/4-architecture/features/**/*.md : Publish prompt and tool usage notes alongside the feature plan update so future agents understand how the iteration unfolded.Applied to files:
docs/specs/4-architecture/features/004-album-size-statistics/plan.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Applies to docs/specs/4-architecture/open-questions.md : Update or close entries in docs/specs/4-architecture/open-questions.md after completing work.Applied to files:
docs/specs/4-architecture/open-questions.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Applies to docs/specs/4-architecture/open-questions.md : Capture every high-impact clarification question and each medium-impact uncertainty per feature in docs/specs/4-architecture/open-questions.md and, once resolved, record the outcome directly in the spec (requirements, NFR, behaviour/UI, telemetry/policy sections).Applied to files:
docs/specs/4-architecture/open-questions.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Applies to docs/specs/4-architecture/open-questions.md : Log high- and medium-impact open questions in docs/specs/4-architecture/open-questions.md and remove each row as soon as it is resolved, ensuring the answer is captured first in the governing spec's normative sections.Applied to files:
docs/specs/4-architecture/open-questions.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Applies to docs/specs/6-decisions/**/*.md : For architecturally significant clarifications (cross-feature/module boundaries, security/telemetry strategy, major NFR trade-offs), create or update an ADR using docs/specs/templates/adr-template.md, then mark the corresponding open-questions row as resolved with links to the spec sections and ADR ID.Applied to files:
docs/specs/4-architecture/open-questions.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Applies to docs/specs/6-decisions/**/*.md : Record only confirmed architectural decisions and architecturally significant clarifications as ADRs under docs/specs/6-decisions/ using the adr-template.md, and reference those ADR IDs from the relevant spec sections and open-questions log.Applied to files:
docs/specs/4-architecture/open-questions.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Whenever presenting alternative approaches, first capture or update the entry in docs/specs/4-architecture/open-questions.md (summary row + Question Details section), then present the stored text to the user inline in chat with question ID and numbered-heading/Options A, B, C format with pros/cons, ordered by preference.Applied to files:
docs/specs/4-architecture/open-questions.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Clarify ambiguity first before planning or implementing. Ask the user to record unresolved items in docs/specs/4-architecture/open-questions.md and wait for answers. Update relevant specifications to be the single source of truth for behaviour.Applied to files:
docs/specs/4-architecture/open-questions.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Applies to docs/specs/5-decisions/**/*.md : Before planning or implementation, skim ADRs under docs/specs/5-decisions whose related-features/specs entries reference the active feature ID so high-impact clarifications and architectural decisions are treated as required context.Applied to files:
docs/specs/4-architecture/open-questions.md📚 Learning: 2025-12-22T14:12:18.082Z
Learnt from: CR Repo: LycheeOrg/Lychee PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-22T14:12:18.082Z Learning: Never ask the user for clarification, approval, or a decision in chat until the matching open question is logged in docs/specs/4-architecture/open-questions.md (table row + Question Details entry). Treat violations as blockers.Applied to files:
docs/specs/4-architecture/open-questions.md📚 Learning: 2025-09-16T21:56:01.607Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3683 File: app/Actions/Shop/OrderService.php:65-73 Timestamp: 2025-09-16T21:56:01.607Z Learning: The validation for album-photo membership in PurchasableService::getEffectivePurchasableForPhoto() uses a JOIN query with the photo_album pivot table to ensure that only purchasables for albums that actually contain the specified photo are returned, preventing price spoofing attacks where clients could use arbitrary album_ids.Applied to files:
app/Actions/Statistics/Spaces.php📚 Learning: 2025-09-16T21:56:01.607Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3683 File: app/Actions/Shop/OrderService.php:65-73 Timestamp: 2025-09-16T21:56:01.607Z Learning: The validation for album-photo membership in OrderService::addPhotoToOrder() is implemented within PurchasableService::getEffectivePurchasableForPhoto() using a whereHas('albums') constraint that ensures the photo belongs to the specified album_id before applying pricing logic.Applied to files:
app/Actions/Statistics/Spaces.php📚 Learning: 2025-09-13T14:07:11.118Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3683 File: app/Http/Controllers/Shop/CatalogController.php:48-52 Timestamp: 2025-09-13T14:07:11.118Z Learning: In CatalogController, the children_purchasables query intentionally uses parent_id to fetch only direct children albums, not the entire subtree via nested set bounds. This is a design decision to show purchasables from immediate child albums only.Applied to files:
app/Actions/Statistics/Spaces.php🧬 Code graph analysis (12)
app/Listeners/RecomputeAlbumSizeOnPhotoMutation.php (3)
app/Events/PhotoDeleted.php (1)
PhotoDeleted(14-27)app/Events/PhotoSaved.php (1)
PhotoSaved(15-26)app/Jobs/RecomputeAlbumSizeJob.php (1)
RecomputeAlbumSizeJob(32-195)app/Http/Controllers/Admin/Maintenance/BackfillAlbumSizes.php (1)
app/Jobs/RecomputeAlbumSizeJob.php (1)
RecomputeAlbumSizeJob(32-195)app/Models/AlbumSizeStatistics.php (1)
app/Models/Album.php (1)
Album(145-532)tests/Precomputing/RecomputeAlbumSizeJobTest.php (4)
app/Jobs/RecomputeAlbumSizeJob.php (1)
RecomputeAlbumSizeJob(32-195)tests/Precomputing/Base/BasePrecomputingTest.php (1)
BasePrecomputingTest(30-78)database/factories/AlbumFactory.php (1)
as_root(73-78)database/factories/SizeVariantFactory.php (3)
for_photo(71-76)type(85-111)with_size(120-125)tests/Precomputing/AlbumSizePropagationTest.php (6)
app/Jobs/RecomputeAlbumSizeJob.php (1)
RecomputeAlbumSizeJob(32-195)app/Models/AlbumSizeStatistics.php (1)
AlbumSizeStatistics(39-119)tests/Precomputing/Base/BasePrecomputingTest.php (1)
BasePrecomputingTest(30-78)database/factories/AlbumFactory.php (2)
as_root(73-78)children_of(57-66)app/Services/Auth/SessionOrTokenGuard.php (1)
id(227-230)bootstrap/PanicAttack.php (1)
root(60-67)database/factories/SizeVariantFactory.php (1)
app/Models/SizeVariant.php (1)
photo(147-150)app/Console/Commands/RecomputeAlbumSizes.php (2)
app/Jobs/RecomputeAlbumSizeJob.php (1)
RecomputeAlbumSizeJob(32-195)app/Console/Commands/RecomputeAlbumStats.php (1)
RecomputeAlbumStats(16-80)routes/api_v2.php (1)
app/Http/Controllers/Admin/Maintenance/BackfillAlbumSizes.php (1)
BackfillAlbumSizes(33-91)app/Models/Album.php (1)
app/Models/AlbumSizeStatistics.php (1)
AlbumSizeStatistics(39-119)tests/Unit/Models/AlbumSizeStatisticsTest.php (2)
app/Models/AlbumSizeStatistics.php (1)
AlbumSizeStatistics(39-119)app/Models/Album.php (1)
sizeStatistics(284-287)app/Providers/EventServiceProvider.php (4)
app/Listeners/RecomputeAlbumSizeOnPhotoMutation.php (1)
RecomputeAlbumSizeOnPhotoMutation(25-70)app/Listeners/RecomputeAlbumSizeOnVariantMutation.php (1)
RecomputeAlbumSizeOnVariantMutation(24-86)app/Events/PhotoSaved.php (1)
PhotoSaved(15-26)app/Events/PhotoDeleted.php (1)
PhotoDeleted(14-27)app/Actions/Statistics/Spaces.php (1)
app/Eloquent/FixedQueryBuilderTrait.php (2)
select(134-144)leftJoin(211-221)🪛 LanguageTool
docs/specs/4-architecture/features/004-album-size-statistics/spec.md
[style] ~37-~37: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...n:Propagating to parent {parent_id}. Log propagation stop: `Propagation stopped ...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: 2️⃣ PHP dist / 8.5 - mariadb
- GitHub Check: 2️⃣ PHP dist / 8.4 - mariadb
- GitHub Check: 2️⃣ PHP dist / 8.4 - postgresql
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Install
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Install
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Precomputing
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/Unit/Actions/Db/OptimizeDbTest.php (1)
27-29: Correct the outdated comment.The comment describes iterating over directories and checking license headers, but the test actually validates the return count from
OptimizeDb::do(). Additionally, there's a typo with the double period.🔎 Proposed fix
- /** - * Iterate over the directories and check if the files contain the correct license and copyright info.. - * - * @return void - */ + /** + * Test that OptimizeDb returns the expected number of optimizations performed. + * + * @return void + */tests/Unit/Actions/Db/OptimizeTablesTest.php (1)
27-29: Fix the incorrect PHPDoc comment.The comment describes checking files for license/copyright information, but this test actually verifies database table optimization. This appears to be a copy-paste error from another test.
🔎 Proposed fix
/** - * Iterate over the directories and check if the files contain the correct license and copyright info.. + * Test that OptimizeTables returns the expected number of optimized tables. * * @return void */
🧹 Nitpick comments (1)
tests/Unit/Actions/Db/OptimizeDbTest.php (1)
35-35: Consider adding context for the expected values.The test asserts against specific magic numbers (3, 36, 37) without explaining what they represent or why these particular values are expected. This makes the test harder to maintain and understand.
Consider adding a comment explaining:
- What each value represents (e.g., number of tables optimized in different scenarios)
- Why the value changed from 35 to 37 in this PR
Additionally, if these values represent different optimization scenarios, consider using named constants or splitting into separate test cases for clarity.
📋 Example improvement
public function testOptimizeDb(): void { $optimize = new OptimizeDb(); $output = count($optimize->do()); + // Expected counts: + // - 3: Minimal tables in clean install + // - 36: Standard tables after migrations + // - 37: Standard tables + album_sizes (after adding precomputed statistics) self::assertTrue(in_array($output, [3, 36, 37], true), 'OptimizeDb should return either 3 or 36 or 37: ' . $output); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/Metadata/Cache/RouteCacheManager.phptests/Unit/Actions/Db/OptimizeDbTest.phptests/Unit/Actions/Db/OptimizeTablesTest.php
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Any new PHP file should contain the license header and have a single blank line after the opening PHP tag
Variable names should be in snake_case in PHP
Apply the PSR-4 coding standard in PHP
Use in_array() with true as the third parameter in PHP
Only use booleans in if statements, not integers or strings
Use strict comparison (===) instead of loose comparison (==)
Avoid code duplication in both if and else statements
Do not use empty() in PHP
Use the moneyphp/money library for handling monetary values in PHP
Never use floats or doubles to represent monetary values; use integers representing the smallest currency unit (e.g., cents for USD)
**/*.php: Write or extend executable specifications (unit, behaviour, or scenario tests) ahead of implementation, confirm they fail, and then drive code to green before refactoring. List the expected success, validation, and failure branches and add thin failing tests for each path.
For PHP code, adhere to conventions: license headers in new files, strict comparison (===), no empty(), in_array() with third parameter true, snake_case variables, PSR-4 standard, test base classes (AbstractTestCase for Unit, BaseApiWithDataTest for Feature_v2).
Always run phpunit tests. If a test remains red, disable it with a TODO, note the reason, and capture the follow-up in the relevant plan.
Spotless now uses Palantir Java Format 2.78.0 with a 120-character wrap; configure IDE formatters to match before pushing code changes.
Keep each increment's control flow flat by delegating validation/normalisation into tiny pure helpers that return simple enums or result records, then compose them instead of introducing inline branching that inflates the branch count per change.
When introducing new helpers/utilities or editing files prone to style violations (records, DTOs, generated adapters), run the narrowest applicable lint target (for example phpstan) before the full pipeline. Note the command in the related plan/task.
For PHP changes, ru...
Files:
tests/Unit/Actions/Db/OptimizeTablesTest.phpapp/Metadata/Cache/RouteCacheManager.phptests/Unit/Actions/Db/OptimizeDbTest.php
tests/Unit/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests in the tests/Unit directory should extend from AbstractTestCase
Files:
tests/Unit/Actions/Db/OptimizeTablesTest.phptests/Unit/Actions/Db/OptimizeDbTest.php
tests/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
No need to mock the database in tests; use the in-memory SQLite database instead
Files:
tests/Unit/Actions/Db/OptimizeTablesTest.phptests/Unit/Actions/Db/OptimizeDbTest.php
🧠 Learnings (1)
📚 Learning: 2025-12-28T18:12:55.752Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3901
File: app/Providers/AppServiceProvider.php:0-0
Timestamp: 2025-12-28T18:12:55.752Z
Learning: When using Laravel Octane's tick API, Octane::tick(...) returns an InvokeTickCallable that only has ->seconds(int) and ->immediate() methods. There is no ->every(N) method. Use the correct usage: Octane::tick('name', fn() => ...)->seconds(N) or Octane::tick('name', fn() => ..., N). Apply this guideline to PHP files across the project (not just AppServiceProvider.php).
Applied to files:
tests/Unit/Actions/Db/OptimizeTablesTest.phpapp/Metadata/Cache/RouteCacheManager.phptests/Unit/Actions/Db/OptimizeDbTest.php
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: 2️⃣ PHP dist / 8.4 - mariadb
- GitHub Check: 2️⃣ PHP dist / 8.5 - postgresql
- GitHub Check: 2️⃣ PHP dist / 8.5 - mariadb
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.4 - sqlite -- Install
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Feature_v2
🔇 Additional comments (2)
app/Metadata/Cache/RouteCacheManager.php (1)
90-90: LGTM! Correctly marked as non-cacheable.The new backfill route is appropriately set to
false, ensuring that each maintenance request reflects the current system state. This is consistent with all other maintenance endpoints and follows the established pattern in this file.tests/Unit/Actions/Db/OptimizeTablesTest.php (1)
31-36: The test count change from [3, 35, 36] to [3, 36, 37] is correct. Thealbum_size_statisticstable is confirmed to exist (migration:2026_01_02_203124_create_album_size_statistics_table.php) and is actively used in the codebase. The +1 increment accurately reflects the new table being counted byOptimizeTables::do().
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
app/Actions/Statistics/Spaces.php (1)
176-202: Consider simplifying the manual array mapping.The manual if-block mapping from result fields to the variants array is functional but verbose. Consider refactoring to use a loop over size variant types for improved maintainability.
🔎 Optional refactoring to reduce repetition
$size_map = [ SizeVariantType::ORIGINAL => 'size_original', SizeVariantType::MEDIUM2X => 'size_medium2x', SizeVariantType::MEDIUM => 'size_medium', SizeVariantType::SMALL2X => 'size_small2x', SizeVariantType::SMALL => 'size_small', SizeVariantType::THUMB2X => 'size_thumb2x', SizeVariantType::THUMB => 'size_thumb', ]; $variants = []; foreach ($size_map as $type => $field) { $size = intval($result->$field ?? 0); if ($size > 0) { $variants[] = ['type' => $type, 'size' => $size]; } } return collect($variants);lang/sk/maintenance.php (1)
98-103: Missing Slovak translation for new entry.The
backfill-album-sizesentry uses English text. For consistency with the translated portions of this file, consider providing Slovak translations fortitle,description, andbutton.Note: This follows the existing pattern where some entries remain untranslated (e.g., lines 63-72, 78-97), so this can be addressed later.
lang/zh_CN/maintenance.php (1)
98-102: Missing Chinese translation for new entry.The
backfill-album-sizesentry uses English text. For consistency with the translated portions of this file (e.g., lines 9-10, 11-16), consider providing Chinese translations.lang/fr/maintenance.php (1)
98-102: Missing French translation for new entry.The
backfill-album-sizesentry uses English text. Since most of this file is translated to French (lines 9-77), consider providing French translations for consistency.app/Jobs/RecomputeAlbumSizeJob.php (1)
161-168: Use strict comparison for enum value.Per coding guidelines, use strict comparison (
!==) instead of loose comparison (!=).🔎 Proposed fix
$results = DB::table('size_variants') ->join('photos', 'size_variants.photo_id', '=', 'photos.id') ->join(PA::PHOTO_ALBUM, 'photos.id', '=', PA::PHOTO_ID) ->where(PA::ALBUM_ID, '=', $album->id) - ->where('size_variants.type', '!=', SizeVariantType::PLACEHOLDER->value) + ->where('size_variants.type', '!==', SizeVariantType::PLACEHOLDER->value) ->select('size_variants.type', DB::raw('SUM(size_variants.filesize) as total_size')) ->groupBy('size_variants.type') ->get();tests/Precomputing/SizeComputations/RecomputeAlbumSizeJobTest.php (1)
264-272: Consider stronger assertion for graceful handling test.The
assertTrue(true)assertion at line 271 is a placeholder that doesn't verify actual behavior. Consider using$this->addToAssertionCount(1)to explicitly mark this as a "no exception thrown" test, or add a comment explaining the test validates execution completes without exception.🔎 Proposed improvement
public function testHandlesMissingAlbum(): void { // Run job with non-existent album ID $job = new RecomputeAlbumSizeJob('nonexistent-id'); $job->handle(); - // Should not throw exception, job logs warning and returns - $this->assertTrue(true); + // Should not throw exception, job logs warning and returns. + // Reaching this point without exception proves graceful handling. + $this->addToAssertionCount(1); }tests/Precomputing/SizeComputations/AlbumSizePropagationTest.php (2)
143-143: Inconsistent method naming.The method name
testNopropagationForRootAlbumshould use consistent camelCase:testNoPropagationForRootAlbum.🔎 Proposed fix
- public function testNopropagationForRootAlbum(): void + public function testNoPropagationForRootAlbum(): void
111-136: Strengthen failure propagation test.The test uses
assertTrue(true)which doesn't verify actual behavior. SinceQueue::fake()is already set up, consider asserting that no jobs were dispatched afterfailed()is called.🔎 Proposed improvement
// Simulate failure by calling failed() method $exception = new \Exception('Database connection lost'); $job->failed($exception); - // Verify that failed() method handles the error gracefully - // In a real scenario, if handle() throws an exception before dispatching - // the parent job, propagation stops automatically - $this->assertTrue(true, 'failed() method handles propagation stop correctly'); + // Verify that failed() method doesn't dispatch any propagation jobs + Queue::assertNothingPushed(); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
app/Actions/Statistics/Spaces.phpapp/Console/Commands/RecomputeAlbumSizes.phpapp/Console/Commands/RecomputeAlbumStats.phpapp/Http/Controllers/Admin/Maintenance/BackfillAlbumSizes.phpapp/Http/Controllers/Admin/Maintenance/FulfillPreCompute.phpapp/Jobs/RecomputeAlbumSizeJob.phpapp/Jobs/RecomputeAlbumStatsJob.phplang/ar/maintenance.phplang/cz/maintenance.phplang/de/maintenance.phplang/el/maintenance.phplang/en/maintenance.phplang/es/maintenance.phplang/fa/maintenance.phplang/fr/maintenance.phplang/hu/maintenance.phplang/it/maintenance.phplang/ja/maintenance.phplang/nl/maintenance.phplang/no/maintenance.phplang/pl/maintenance.phplang/pt/maintenance.phplang/ru/maintenance.phplang/sk/maintenance.phplang/sv/maintenance.phplang/vi/maintenance.phplang/zh_CN/maintenance.phplang/zh_TW/maintenance.phptests/Precomputing/CoverSelection/RecomputeAlbumStatsCommandTest.phptests/Precomputing/SizeComputations/AlbumSizeEventListenerTest.phptests/Precomputing/SizeComputations/AlbumSizePropagationTest.phptests/Precomputing/SizeComputations/RecomputeAlbumSizeJobTest.php
💤 Files with no reviewable changes (1)
- tests/Precomputing/CoverSelection/RecomputeAlbumStatsCommandTest.php
🚧 Files skipped from review as they are similar to previous changes (14)
- lang/cz/maintenance.php
- lang/ru/maintenance.php
- lang/nl/maintenance.php
- lang/ja/maintenance.php
- lang/el/maintenance.php
- lang/en/maintenance.php
- lang/sv/maintenance.php
- lang/hu/maintenance.php
- app/Http/Controllers/Admin/Maintenance/BackfillAlbumSizes.php
- lang/pl/maintenance.php
- lang/vi/maintenance.php
- lang/es/maintenance.php
- lang/de/maintenance.php
- lang/zh_TW/maintenance.php
🧰 Additional context used
📓 Path-based instructions (2)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Any new PHP file should contain the license header and have a single blank line after the opening PHP tag
Variable names should be in snake_case in PHP
Apply the PSR-4 coding standard in PHP
Use in_array() with true as the third parameter in PHP
Only use booleans in if statements, not integers or strings
Use strict comparison (===) instead of loose comparison (==)
Avoid code duplication in both if and else statements
Do not use empty() in PHP
Use the moneyphp/money library for handling monetary values in PHP
Never use floats or doubles to represent monetary values; use integers representing the smallest currency unit (e.g., cents for USD)
**/*.php: Write or extend executable specifications (unit, behaviour, or scenario tests) ahead of implementation, confirm they fail, and then drive code to green before refactoring. List the expected success, validation, and failure branches and add thin failing tests for each path.
For PHP code, adhere to conventions: license headers in new files, strict comparison (===), no empty(), in_array() with third parameter true, snake_case variables, PSR-4 standard, test base classes (AbstractTestCase for Unit, BaseApiWithDataTest for Feature_v2).
Always run phpunit tests. If a test remains red, disable it with a TODO, note the reason, and capture the follow-up in the relevant plan.
Spotless now uses Palantir Java Format 2.78.0 with a 120-character wrap; configure IDE formatters to match before pushing code changes.
Keep each increment's control flow flat by delegating validation/normalisation into tiny pure helpers that return simple enums or result records, then compose them instead of introducing inline branching that inflates the branch count per change.
When introducing new helpers/utilities or editing files prone to style violations (records, DTOs, generated adapters), run the narrowest applicable lint target (for example phpstan) before the full pipeline. Note the command in the related plan/task.
For PHP changes, ru...
Files:
lang/fr/maintenance.phpapp/Actions/Statistics/Spaces.phplang/pt/maintenance.phpapp/Jobs/RecomputeAlbumStatsJob.phplang/zh_CN/maintenance.phplang/no/maintenance.phplang/it/maintenance.phplang/sk/maintenance.phpapp/Http/Controllers/Admin/Maintenance/FulfillPreCompute.phpapp/Console/Commands/RecomputeAlbumStats.phplang/ar/maintenance.phptests/Precomputing/SizeComputations/AlbumSizeEventListenerTest.phptests/Precomputing/SizeComputations/AlbumSizePropagationTest.phptests/Precomputing/SizeComputations/RecomputeAlbumSizeJobTest.phpapp/Console/Commands/RecomputeAlbumSizes.phpapp/Jobs/RecomputeAlbumSizeJob.phplang/fa/maintenance.php
tests/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
No need to mock the database in tests; use the in-memory SQLite database instead
Files:
tests/Precomputing/SizeComputations/AlbumSizeEventListenerTest.phptests/Precomputing/SizeComputations/AlbumSizePropagationTest.phptests/Precomputing/SizeComputations/RecomputeAlbumSizeJobTest.php
🧠 Learnings (9)
📓 Common learnings
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3922
File: app/Listeners/RecomputeAlbumSizeOnPhotoMutation.php:63-69
Timestamp: 2026-01-03T12:36:02.514Z
Learning: In Lychee, "deleting a photo from an album" (removing the photo-album association) is distinct from "deleting a photo entirely". When PhotoDeleted event is dispatched with a specific album_id, it indicates the photo was removed from that particular album only. The photo may still exist in other albums due to the many-to-many relationship, so only the specified album's size statistics need recomputation.
📚 Learning: 2025-12-19T21:01:32.168Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3838
File: lang/pl/webshop.php:1-2
Timestamp: 2025-12-19T21:01:32.168Z
Learning: In the Lychee repository, PHP files under the lang/ directory (and its subdirectories) do not require the standard license header. This is an exception to the general PHP license header rule. Ensure all non-lang PHP files continue to include the license header.
Applied to files:
lang/fr/maintenance.phplang/pt/maintenance.phplang/zh_CN/maintenance.phplang/no/maintenance.phplang/it/maintenance.phplang/sk/maintenance.phplang/ar/maintenance.phplang/fa/maintenance.php
📚 Learning: 2025-12-19T21:01:45.910Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3838
File: lang/ar/webshop.php:1-2
Timestamp: 2025-12-19T21:01:45.910Z
Learning: Do not require license headers for language translation files under the lang/ directory (e.g., lang/ar/webshop.php). These resource files are exempt from header checks; apply header enforcement to other PHP source files in the repo.
Applied to files:
lang/fr/maintenance.phplang/pt/maintenance.phplang/zh_CN/maintenance.phplang/no/maintenance.phplang/it/maintenance.phplang/sk/maintenance.phplang/ar/maintenance.phplang/fa/maintenance.php
📚 Learning: 2025-12-28T18:12:55.752Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3901
File: app/Providers/AppServiceProvider.php:0-0
Timestamp: 2025-12-28T18:12:55.752Z
Learning: When using Laravel Octane's tick API, Octane::tick(...) returns an InvokeTickCallable that only has ->seconds(int) and ->immediate() methods. There is no ->every(N) method. Use the correct usage: Octane::tick('name', fn() => ...)->seconds(N) or Octane::tick('name', fn() => ..., N). Apply this guideline to PHP files across the project (not just AppServiceProvider.php).
Applied to files:
lang/fr/maintenance.phpapp/Actions/Statistics/Spaces.phplang/pt/maintenance.phpapp/Jobs/RecomputeAlbumStatsJob.phplang/zh_CN/maintenance.phplang/no/maintenance.phplang/it/maintenance.phplang/sk/maintenance.phpapp/Http/Controllers/Admin/Maintenance/FulfillPreCompute.phpapp/Console/Commands/RecomputeAlbumStats.phplang/ar/maintenance.phptests/Precomputing/SizeComputations/AlbumSizeEventListenerTest.phptests/Precomputing/SizeComputations/AlbumSizePropagationTest.phptests/Precomputing/SizeComputations/RecomputeAlbumSizeJobTest.phpapp/Console/Commands/RecomputeAlbumSizes.phpapp/Jobs/RecomputeAlbumSizeJob.phplang/fa/maintenance.php
📚 Learning: 2025-09-16T21:56:01.607Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:65-73
Timestamp: 2025-09-16T21:56:01.607Z
Learning: The validation for album-photo membership in PurchasableService::getEffectivePurchasableForPhoto() uses a JOIN query with the photo_album pivot table to ensure that only purchasables for albums that actually contain the specified photo are returned, preventing price spoofing attacks where clients could use arbitrary album_ids.
Applied to files:
app/Actions/Statistics/Spaces.php
📚 Learning: 2025-09-16T21:56:01.607Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:65-73
Timestamp: 2025-09-16T21:56:01.607Z
Learning: The validation for album-photo membership in OrderService::addPhotoToOrder() is implemented within PurchasableService::getEffectivePurchasableForPhoto() using a whereHas('albums') constraint that ensures the photo belongs to the specified album_id before applying pricing logic.
Applied to files:
app/Actions/Statistics/Spaces.php
📚 Learning: 2025-09-13T14:07:11.118Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Http/Controllers/Shop/CatalogController.php:48-52
Timestamp: 2025-09-13T14:07:11.118Z
Learning: In CatalogController, the children_purchasables query intentionally uses parent_id to fetch only direct children albums, not the entire subtree via nested set bounds. This is a design decision to show purchasables from immediate child albums only.
Applied to files:
app/Actions/Statistics/Spaces.php
📚 Learning: 2025-08-22T06:11:18.329Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3641
File: lang/no/settings.php:9-9
Timestamp: 2025-08-22T06:11:18.329Z
Learning: For lang/* translation files in the Lychee project: only review PHP-related issues (syntax, structure, etc.), not translation content, grammar, or language-related nitpicks. The maintainer ildyria has explicitly requested this approach.
Applied to files:
lang/ar/maintenance.phplang/fa/maintenance.php
📚 Learning: 2026-01-03T12:36:02.514Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3922
File: app/Listeners/RecomputeAlbumSizeOnPhotoMutation.php:63-69
Timestamp: 2026-01-03T12:36:02.514Z
Learning: In Lychee, "deleting a photo from an album" (removing the photo-album association) is distinct from "deleting a photo entirely". When PhotoDeleted event is dispatched with a specific album_id, it indicates the photo was removed from that particular album only. The photo may still exist in other albums due to the many-to-many relationship, so only the specified album's size statistics need recomputation.
Applied to files:
tests/Precomputing/SizeComputations/AlbumSizeEventListenerTest.phpapp/Jobs/RecomputeAlbumSizeJob.php
🧬 Code graph analysis (7)
app/Actions/Statistics/Spaces.php (2)
resources/js/lychee.d.ts (1)
SizeVariantType(118-118)app/Enum/Traits/DecorateBackedEnum.php (1)
values(46-49)
app/Http/Controllers/Admin/Maintenance/FulfillPreCompute.php (1)
app/Jobs/RecomputeAlbumStatsJob.php (1)
RecomputeAlbumStatsJob(37-365)
app/Console/Commands/RecomputeAlbumStats.php (1)
app/Jobs/RecomputeAlbumStatsJob.php (1)
RecomputeAlbumStatsJob(37-365)
tests/Precomputing/SizeComputations/AlbumSizeEventListenerTest.php (5)
app/Actions/Photo/MoveOrDuplicate.php (1)
MoveOrDuplicate(23-136)app/Jobs/RecomputeAlbumSizeJob.php (2)
RecomputeAlbumSizeJob(32-194)handle(96-132)app/Models/AlbumSizeStatistics.php (1)
AlbumSizeStatistics(39-119)app/Models/SizeVariant.php (1)
SizeVariant(71-239)app/Events/PhotoSaved.php (1)
PhotoSaved(15-26)
tests/Precomputing/SizeComputations/AlbumSizePropagationTest.php (3)
app/Jobs/RecomputeAlbumSizeJob.php (3)
RecomputeAlbumSizeJob(32-194)handle(96-132)failed(189-193)app/Models/AlbumSizeStatistics.php (1)
AlbumSizeStatistics(39-119)app/Jobs/RecomputeAlbumStatsJob.php (2)
handle(101-155)failed(360-364)
tests/Precomputing/SizeComputations/RecomputeAlbumSizeJobTest.php (4)
app/Jobs/RecomputeAlbumSizeJob.php (2)
RecomputeAlbumSizeJob(32-194)handle(96-132)app/Models/AlbumSizeStatistics.php (1)
AlbumSizeStatistics(39-119)database/factories/AlbumFactory.php (1)
as_root(73-78)database/factories/SizeVariantFactory.php (3)
for_photo(71-76)type(85-111)with_size(120-125)
app/Console/Commands/RecomputeAlbumSizes.php (2)
app/Jobs/RecomputeAlbumSizeJob.php (2)
RecomputeAlbumSizeJob(32-194)handle(96-132)app/Console/Commands/RecomputeAlbumStats.php (3)
handle(39-49)handleSingleAlbum(54-94)handleBulkBackfill(99-168)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: 3️⃣ Dockerfile Lint
- GitHub Check: 2️⃣ PHP dist / 8.5 - mariadb
- GitHub Check: 2️⃣ PHP dist / 8.4 - sqlite
- GitHub Check: 2️⃣ PHP dist / 8.4 - mariadb
- GitHub Check: 2️⃣ PHP dist / 8.5 - sqlite
- GitHub Check: 2️⃣ PHP dist / 8.5 - postgresql
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- ImageProcessing
🔇 Additional comments (28)
lang/ar/maintenance.php (1)
97-101: LGTM! Structure and syntax are correct.The new translation entry follows the established pattern in the file with proper array structure, consistent formatting, and correct PHP syntax.
lang/fa/maintenance.php (1)
97-101: LGTM! The PHP syntax and structure are correct.The new translation entry follows the same structure as other maintenance entries in the file (e.g.,
fulfill-precomputeat lines 87-91), with proper array syntax, string placeholders (%d), and HTML formatting consistent with the rest of the file.lang/it/maintenance.php (1)
99-103: Translate to Italian.This file is the Italian translation resource (
lang/it/), but the new entries are in English. The translations should be in Italian for Italian-speaking users.🔎 Suggested Italian translations
'backfill-album-sizes' => [ - 'title' => 'Album Size Statistics', - 'description' => 'Found %d albums without size statistics.<br/><br/>Equivalent to running: php artisan lychee:backfill-album-sizes', - 'button' => 'Compute sizes', + 'title' => 'Statistiche Dimensioni Album', + 'description' => 'Trovati %d album senza statistiche sulle dimensioni.<br/><br/>Equivalente a eseguire: php artisan lychee:backfill-album-sizes', + 'button' => 'Calcola dimensioni', ],⛔ Skipped due to learnings
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3641 File: lang/no/settings.php:9-9 Timestamp: 2025-08-22T06:11:18.329Z Learning: For lang/* translation files in the Lychee project: only review PHP-related issues (syntax, structure, etc.), not translation content, grammar, or language-related nitpicks. The maintainer ildyria has explicitly requested this approach.Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3654 File: lang/cz/gallery.php:210-210 Timestamp: 2025-08-27T08:48:27.520Z Learning: For this Lychee project, the maintainer prefers to keep language strings in English across all locale files rather than translating them to local languages.Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3654 File: lang/pl/gallery.php:210-210 Timestamp: 2025-08-27T08:48:32.956Z Learning: The user ildyria does not prioritize strict localization consistency for new menu items in language files, as indicated by their "Lang = don't care" response when suggested to translate 'Import from Server' to Polish in lang/pl/gallery.php.lang/pt/maintenance.php (1)
99-103: Translate the new entry to Portuguese.The new
backfill-album-sizesentry is in English, but this is a Portuguese translation file (lang/pt/). Portuguese-speaking users will see English text in the maintenance UI.Note: The entire file appears to contain untranslated English content, suggesting a broader localization issue. At minimum, the new entry should be translated.
🔎 Suggested Portuguese translation
'backfill-album-sizes' => [ - 'title' => 'Album Size Statistics', - 'description' => 'Found %d albums without size statistics.<br/><br/>Equivalent to running: php artisan lychee:backfill-album-sizes', - 'button' => 'Compute sizes', + 'title' => 'Estatísticas de Tamanho de Álbum', + 'description' => 'Encontrados %d álbuns sem estatísticas de tamanho.<br/><br/>Equivalente a executar: php artisan lychee:backfill-album-sizes', + 'button' => 'Calcular tamanhos', ],⛔ Skipped due to learnings
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3641 File: lang/no/settings.php:9-9 Timestamp: 2025-08-22T06:11:18.329Z Learning: For lang/* translation files in the Lychee project: only review PHP-related issues (syntax, structure, etc.), not translation content, grammar, or language-related nitpicks. The maintainer ildyria has explicitly requested this approach.app/Jobs/RecomputeAlbumStatsJob.php (1)
56-59: LGTM! Propagation control implemented correctly.The addition of the
propagate_to_parentparameter with a default value oftruemaintains backward compatibility while allowing callers to disable parent propagation for bulk operations. This is a solid design choice to avoid redundant recomputations.app/Console/Commands/RecomputeAlbumStats.php (2)
68-93: LGTM! Dispatch calls updated correctly for single-album mode.The synchronous and asynchronous paths now properly pass
propagate_to_parent=true, which is correct for single-album recomputation where parent propagation is desired.
132-141: LGTM! Bulk backfill correctly disables propagation.The comment and code correctly disable parent propagation (
false) for bulk processing. Since albums are processed in leaf-to-root order (by_lft ASC), each parent will be processed independently, making propagation redundant.app/Http/Controllers/Admin/Maintenance/FulfillPreCompute.php (2)
58-66: LGTM! Synchronous batch processing optimized correctly.The batch size increase from 50 to 100 is a reasonable performance optimization. The propagation flag is correctly set to
falseto avoid redundant parent recomputation jobs, which is appropriate for bulk operations where all albums are being processed.
67-76: LGTM! Asynchronous processing correctly disables propagation.The async path also correctly disables propagation for the same reason as the sync path. The comment clearly explains the rationale.
app/Actions/Statistics/Spaces.php (6)
27-65: LGTM! User space calculation optimized with pre-computed statistics.The refactor correctly leverages the
album_size_statisticstable with proper null handling viaCOALESCE. The leftJoin approach ensures users without albums still appear in results with zero size.
67-142: LGTM! Two-query approach correctly handles albummed and unalbummed photos.The split query strategy is sound:
- Query 1 aggregates sizes from
album_size_statisticsfor photos in albums- Query 2 aggregates sizes from
size_variantsfor photos not in any album (viawhereNotExists)- Results are combined by
SizeVariantTypeThis ensures accurate size reporting for all photos regardless of album membership.
205-262: LGTM! Album space calculation correctly uses pre-computed statistics.The query properly joins
album_size_statisticsand aggregates all size fields usingCOALESCEfor null handling. The nested set filtering for album scope is correct.
264-318: LGTM! Total space calculation correctly includes descendants.The nested set join correctly identifies all descendant albums, and the SUM aggregation properly computes total space across the album tree.
320-383: LGTM! Photo count now uses pre-computed num_photos column.The refactor correctly leverages the
albums.num_photoscolumn, which is maintained by the recomputation jobs. This is a significant performance improvement over counting photos at runtime.
385-453: LGTM! Total photo count correctly aggregates descendants.The nested set join with
SUM(descendants.num_photos)correctly computes the total count across the album tree.lang/no/maintenance.php (1)
97-101: LGTM! Translation entry added correctly.The new
backfill-album-sizesmaintenance entry follows the established pattern with title, description, and button text. The description helpfully references the equivalent CLI command.app/Jobs/RecomputeAlbumSizeJob.php (2)
96-132: LGTM!The
handle()method correctly:
- Clears the cache entry before processing (allowing new jobs to register)
- Validates album existence with early return
- Uses
updateOrCreatefor idempotent statistics storage- Propagates to parent only when
propagate_to_parentis true- Properly re-throws exceptions to trigger retry mechanism
189-193: LGTM!The
failed()method correctly stops propagation on failure, preventing cascading issues up the album tree.app/Console/Commands/RecomputeAlbumSizes.php (2)
54-94: LGTM!The
handleSingleAlbum()method correctly:
- Validates album existence with strict null check
- Supports both synchronous and asynchronous execution modes
- Provides appropriate user feedback and logging
- Handles exceptions gracefully in sync mode
99-167: LGTM!The
handleBulkBackfill()method correctly:
- Uses
_lft ASCordering for leaf-to-root processing (correct for nested set)- Disables parent propagation (
propagate_to_parent = false) to avoid redundant work- Uses chunked processing for memory efficiency
- Provides progress indication with periodic logging
tests/Precomputing/SizeComputations/RecomputeAlbumSizeJobTest.php (1)
24-54: LGTM!Excellent test coverage for
RecomputeAlbumSizeJob. The tests comprehensively verify:
- Basic size computation with all variant types
- PLACEHOLDER variant exclusion
- Empty album handling
- Partial variant scenarios
- Multi-photo aggregation
- Idempotent update-or-create behavior
- Parent propagation mechanics
- Direct children vs descendants distinction
tests/Precomputing/SizeComputations/AlbumSizePropagationTest.php (1)
25-75: LGTM!Comprehensive propagation test suite covering:
- Three-level nesting with bottom-up recomputation
- Job dispatch verification via
Queue::fake()- Root album (no parent) scenarios
- Branching tree structures
- Multiple mutations with aggregation
- Deep nesting (5 levels)
The tests properly validate that parent albums have zero direct-photo sizes while child albums reflect actual photo data.
tests/Precomputing/SizeComputations/AlbumSizeEventListenerTest.php (6)
1-29: LGTM! Proper test setup and structure.The license header, namespace, imports, and class declaration follow PSR-4 standards and coding guidelines. The test extends the appropriate base class for precomputing tests.
36-78: LGTM! Event dispatch tests are well-structured.The tests correctly use
Queue::fake()to verify job dispatching, and properly test photo creation and deletion scenarios. The use of strict comparison (===) in assertions follows coding guidelines.
85-108: LGTM! Move operation test correctly verifies both albums.The test properly validates that moving a photo between albums triggers recomputation jobs for both the source and destination albums, which aligns with the event dispatching logic in
MoveOrDuplicate.Based on learnings, the PhotoDeleted event with album_id correctly triggers recomputation only for the source album.
117-178: Verify that variant event implementation is tracked.Both
testVariantRegenerationManualRecomputationandtestVariantDeletionManualRecomputationinclude notes that "SizeVariant events not yet implemented." These tests manually trigger recomputation as a workaround.Ensure there's a tracking issue or TODO for implementing automatic event-driven recomputation when size variants are created, updated, or deleted.
185-206: LGTM! Important edge case coverage for multi-album photos.This test correctly verifies that when a photo belongs to multiple albums, the PhotoSaved event triggers recomputation jobs for all affected albums. This is crucial for maintaining accurate statistics across the album hierarchy.
213-299: LGTM! Comprehensive end-to-end test coverage.The end-to-end tests provide excellent integration coverage by:
- Actually executing job logic (not using Queue::fake)
- Verifying statistics are created and updated correctly in the database
- Testing the complete flow from photo operations to statistics updates
This complements the event dispatch tests nicely, providing both unit-level (event dispatch) and integration-level (full execution) confidence.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
app/Actions/Import/Pipes/ImportPhotos.php (1)
85-90: Remove redundant null-safe operator inside null-checked block.Line 87 already verifies that
$node->album !== null, so using the null-safe operator (?->) on lines 88-89 is redundant. Use the regular property access operator instead for clarity.🔎 Proposed fix
// Dispatch recompute jobs for the album after importing photos if ($node->album !== null) { - $this->state->job_bus[] = new RecomputeAlbumSizeJob($node->album?->id); - $this->state->job_bus[] = new RecomputeAlbumStatsJob($node->album?->id); + $this->state->job_bus[] = new RecomputeAlbumSizeJob($node->album->id); + $this->state->job_bus[] = new RecomputeAlbumStatsJob($node->album->id); }app/Actions/Import/Pipes/ExecuteBatch.php (1)
38-50: Extract duplicate path computation to a helper method.The logic correctly distinguishes between
ImportImageJobinstances (usingfile_path) and other job types (using class name). However, lines 41 and 46 duplicate the identical ternary expression, violating the DRY principle.🔎 Proposed refactor to eliminate duplication
+ /** + * Get a display path for the job, using file_path for ImportImageJob or class name otherwise. + */ + private function getJobPath(object $job): string + { + return ($job instanceof ImportImageJob) ? $job->file_path : get_class($job); + } + public function handle(ImportDTO $state, \Closure $next): ImportDTO { if (!$state->should_execute_jobs) { return $next($state); } $total = count($state->job_bus); $this->report(ImportEventReport::createNotice('imported', null, 'Processing ' . $total . ' photos...')); foreach ($state->job_bus as $idx => $job) { try { $progress = (int) (($idx + 1) * 100 / $total); - $path = ($job instanceof ImportImageJob) ? $job->file_path : get_class($job); + $path = $this->getJobPath($job); $this->report(ImportEventReport::createDebug('imported', $path, 'Processing... ' . $progress . '%')); dispatch($job); // @codeCoverageIgnoreStart } catch (\Throwable $e) { - $path = ($job instanceof ImportImageJob) ? $job->file_path : get_class($job); + $path = $this->getJobPath($job); $this->report(ImportEventReport::createFromException($e, $path)); } // @codeCoverageIgnoreEnd } return $next($state); }
Overall approach is sound.
The enhanced logging provides valuable context for both successful processing and exception handling, making it easier to trace which jobs are being processed.
tests/Traits/RequiresImageHandler.php (1)
35-37: LGTM, but consider standardizing the cache refresh pattern.The re-resolution of
ConfigManagercorrectly ensures fresh configuration state. However,setUpRequiresGD()(line 48) usesinvalidateCache()instead of re-resolving. For consistency and maintainability, consider standardizing on one approach across both methods.🔎 Suggested refactor to standardize the pattern
Update
setUpRequiresGD()to match the re-resolve pattern:protected function setUpRequiresGD(): void { $config_manager = resolve(ConfigManager::class); $this->hasImagickInit = $config_manager->getValueAsInt(TestConstants::CONFIG_HAS_IMAGICK); Configs::set(TestConstants::CONFIG_HAS_IMAGICK, 0); - $config_manager->invalidateCache(); + // Refresh... + $config_manager = resolve(ConfigManager::class); if ($config_manager->hasImagick()) { static::markTestSkipped('Imagick still enabled although it shouldn\'t. Test Skipped.'); } }app/Console/Commands/RecomputeAlbumSizes.php (2)
66-66: Use consistent wording: "size statistics" instead of "sizes statistics".The messages at lines 66, 74, 79, 90, and 159 use "sizes statistics" (plural + plural), which is grammatically awkward. Use "size statistics" (singular + plural) to match the command description at line 34 and improve clarity.
🔎 Proposed fixes
- $this->info("Recomputing sizes statistics for album: {$album->title} (ID: {$album_id})"); + $this->info("Recomputing size statistics for album: {$album->title} (ID: {$album_id})");- $this->info('✓ Sizes statistics recomputed successfully'); + $this->info('✓ Size statistics recomputed successfully');- $this->error('✗ Failed to recompute sizes statistics: ' . $e->getMessage()); + $this->error('✗ Failed to recompute size statistics: ' . $e->getMessage());- $this->info(' Note: Sizes statistics will be updated when the queue worker processes the job'); + $this->info(' Note: Size statistics will be updated when the queue worker processes the job');- $this->info("Dispatched {$processed} jobs to recompute album sizes statistics"); + $this->info("Dispatched {$processed} jobs to recompute album size statistics");Also applies to: 74-74, 79-79, 90-90, 159-159
132-151: Consider adding a comment explaining toBase() usage.Line 134 uses
toBase()to avoid hydrating full Eloquent models, which is a good performance optimization for bulk processing. However, this means the$albumobjects in line 136 arestdClassinstances, notAlbummodels. Consider adding a brief comment to clarify this for maintainers.🔎 Example clarification
Album::query() ->orderBy('_lft', 'asc') + // Use toBase() to avoid hydrating Eloquent models (only need IDs) ->toBase() ->chunk($chunk_size, function ($albums) use ($dry_run, &$processed, $bar): void { + // Note: $album is stdClass, not Album model foreach ($albums as $album) {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/Actions/Import/Pipes/ExecuteBatch.phpapp/Actions/Import/Pipes/ImportPhotos.phpapp/Console/Commands/RecomputeAlbumSizes.phpapp/DTO/ImportDTO.phptests/Traits/RequiresExifTool.phptests/Traits/RequiresFFMpeg.phptests/Traits/RequiresImageHandler.php
🧰 Additional context used
📓 Path-based instructions (2)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Any new PHP file should contain the license header and have a single blank line after the opening PHP tag
Variable names should be in snake_case in PHP
Apply the PSR-4 coding standard in PHP
Use in_array() with true as the third parameter in PHP
Only use booleans in if statements, not integers or strings
Use strict comparison (===) instead of loose comparison (==)
Avoid code duplication in both if and else statements
Do not use empty() in PHP
Use the moneyphp/money library for handling monetary values in PHP
Never use floats or doubles to represent monetary values; use integers representing the smallest currency unit (e.g., cents for USD)
**/*.php: Write or extend executable specifications (unit, behaviour, or scenario tests) ahead of implementation, confirm they fail, and then drive code to green before refactoring. List the expected success, validation, and failure branches and add thin failing tests for each path.
For PHP code, adhere to conventions: license headers in new files, strict comparison (===), no empty(), in_array() with third parameter true, snake_case variables, PSR-4 standard, test base classes (AbstractTestCase for Unit, BaseApiWithDataTest for Feature_v2).
Always run phpunit tests. If a test remains red, disable it with a TODO, note the reason, and capture the follow-up in the relevant plan.
Spotless now uses Palantir Java Format 2.78.0 with a 120-character wrap; configure IDE formatters to match before pushing code changes.
Keep each increment's control flow flat by delegating validation/normalisation into tiny pure helpers that return simple enums or result records, then compose them instead of introducing inline branching that inflates the branch count per change.
When introducing new helpers/utilities or editing files prone to style violations (records, DTOs, generated adapters), run the narrowest applicable lint target (for example phpstan) before the full pipeline. Note the command in the related plan/task.
For PHP changes, ru...
Files:
app/Actions/Import/Pipes/ImportPhotos.phpapp/Actions/Import/Pipes/ExecuteBatch.phptests/Traits/RequiresFFMpeg.phptests/Traits/RequiresExifTool.phptests/Traits/RequiresImageHandler.phpapp/Console/Commands/RecomputeAlbumSizes.phpapp/DTO/ImportDTO.php
tests/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
No need to mock the database in tests; use the in-memory SQLite database instead
Files:
tests/Traits/RequiresFFMpeg.phptests/Traits/RequiresExifTool.phptests/Traits/RequiresImageHandler.php
🧠 Learnings (3)
📚 Learning: 2025-08-21T09:08:59.512Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3637
File: app/Console/Commands/Sync.php:212-218
Timestamp: 2025-08-21T09:08:59.512Z
Learning: ProcessImageJob is used by the Web API for single photo uploads and only needs photo title renaming (shall_rename_photo_title). ImportImageJob is used by the CLI for bulk imports and receives the full ImportMode with both photo and album renaming flags from the sync command.
Applied to files:
app/Actions/Import/Pipes/ImportPhotos.phpapp/Actions/Import/Pipes/ExecuteBatch.phpapp/DTO/ImportDTO.php
📚 Learning: 2025-12-28T18:12:55.752Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3901
File: app/Providers/AppServiceProvider.php:0-0
Timestamp: 2025-12-28T18:12:55.752Z
Learning: When using Laravel Octane's tick API, Octane::tick(...) returns an InvokeTickCallable that only has ->seconds(int) and ->immediate() methods. There is no ->every(N) method. Use the correct usage: Octane::tick('name', fn() => ...)->seconds(N) or Octane::tick('name', fn() => ..., N). Apply this guideline to PHP files across the project (not just AppServiceProvider.php).
Applied to files:
app/Actions/Import/Pipes/ImportPhotos.phpapp/Actions/Import/Pipes/ExecuteBatch.phptests/Traits/RequiresFFMpeg.phptests/Traits/RequiresExifTool.phptests/Traits/RequiresImageHandler.phpapp/Console/Commands/RecomputeAlbumSizes.phpapp/DTO/ImportDTO.php
📚 Learning: 2025-09-22T12:34:41.046Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3706
File: tests/Unit/Http/Controllers/ImportFromServerControllerTest.php:59-66
Timestamp: 2025-09-22T12:34:41.046Z
Learning: In the LycheeOrg/Lychee project, config changes made via Configs::set() in tests are handled by DatabaseTransactions, so explicit tearDown restoration is not needed for config state.
Applied to files:
tests/Traits/RequiresExifTool.php
🧬 Code graph analysis (4)
app/Actions/Import/Pipes/ImportPhotos.php (2)
app/Jobs/RecomputeAlbumSizeJob.php (1)
RecomputeAlbumSizeJob(32-194)app/Jobs/RecomputeAlbumStatsJob.php (1)
RecomputeAlbumStatsJob(37-365)
app/Actions/Import/Pipes/ExecuteBatch.php (2)
app/Jobs/ImportImageJob.php (1)
ImportImageJob(30-90)app/DTO/ImportEventReport.php (3)
ImportEventReport(15-97)createDebug(52-55)createFromException(57-60)
app/Console/Commands/RecomputeAlbumSizes.php (2)
app/Jobs/RecomputeAlbumSizeJob.php (2)
RecomputeAlbumSizeJob(32-194)handle(96-132)app/Console/Commands/RecomputeAlbumStats.php (4)
handle(39-49)handleSingleAlbum(54-94)handleBulkBackfill(99-168)RecomputeAlbumStats(16-169)
app/DTO/ImportDTO.php (2)
app/Jobs/RecomputeAlbumSizeJob.php (1)
RecomputeAlbumSizeJob(32-194)app/Jobs/RecomputeAlbumStatsJob.php (1)
RecomputeAlbumStatsJob(37-365)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Install
- GitHub Check: 1️⃣ PHP tests / 8.4 - postgresql -- Install
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Feature_v2
- GitHub Check: 1️⃣ PHP 8.4 - Code Style errors
- GitHub Check: 1️⃣ PHP 8.4 - License Check
- GitHub Check: 0️⃣ JS front-end / Node 20
🔇 Additional comments (6)
app/Actions/Import/Pipes/ImportPhotos.php (1)
18-19: LGTM! Imports are correctly added.The new job class imports are properly placed and used in the dispatch logic below.
app/DTO/ImportDTO.php (2)
14-15: LGTM! Job class imports correctly added.The imports are necessary for the updated type annotation on line 28.
28-28: LGTM! Type annotation correctly extended.The docblock accurately reflects that
$job_buscan now containImportImageJob,RecomputeAlbumSizeJob, orRecomputeAlbumStatsJobinstances, matching the usage inImportPhotos.php.app/Actions/Import/Pipes/ExecuteBatch.php (1)
14-14: LGTM: Import added for instanceof checks.The import is correctly added and used in the instanceof checks below.
tests/Traits/RequiresExifTool.php (1)
35-38: LGTM! Config refresh pattern correctly implemented.The re-resolution of
ConfigManagerafter updating the config ensures that thehasExiftool()check sees the updated configuration value, avoiding stale cache issues.tests/Traits/RequiresFFMpeg.php (1)
35-38: LGTM! Consistent config refresh pattern.The re-resolution of
ConfigManagerafter updating the config correctly ensures thathasFFmpeg()reads the fresh configuration value. This matches the pattern established inRequiresExifTool.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
lang/hu/maintenance.php (1)
98-102: LGTM! Structure and format are correct.The new
backfill-album-sizesentry follows the established pattern and is consistent with other maintenance entries in this file. The key naming, structure, placeholder usage, and command reference are all correct.Optional: Consider adding Hungarian translations.
This file is for the Hungarian locale (
lang/hu/), but the entire file contains English text. While the new entry is consistent with the existing content, consider translating all entries to Hungarian for completeness. This is a common gap in open-source projects and can be addressed incrementally.tests/Feature_v2/Statistics/TotalAlbumSpaceTest.php (1)
49-51: Count of 8 is correct for admin user.The test data in
BaseApiWithDataTestcreates exactly 8Albumrecords (not countingTagAlbuminstances):
album1,subAlbum1(owned byuserMayUpload1)album2,subAlbum2(owned byuserMayUpload2)album3(owned byuserNoUpload)album4,subAlbum4(owned byuserLocked)album5(owned byadmin)The endpoint correctly filters by
owner_id. Admin hasmay_administrate = true, soowner_idremainsnull, returning all albums. Non-admin users getowner_idset to their ID, filtering to only their own albums—henceuserMayUpload1correctly returns 2.Optionally, consider adding assertions that verify specific albums returned (e.g., checking
album5->titleis present) rather than only asserting the count. This would make the test more resilient to test data changes.app/Events/PhotoSaved.php (1)
22-22: Event payload changed to use photo ID instead of Photo object.The change from passing a
Photoobject to passing astring $photo_idis correct and aligns with the dispatch site updates. Using an ID instead of the full object reduces serialization overhead for queued listeners.However, note the inconsistency:
AlbumSaved(line 22-24 in AlbumSaved.php) still accepts anAlbumobject, whilePhotoSavednow accepts a string ID. For design consistency, consider whetherAlbumSavedshould also transition to using an album ID.app/Listeners/RecomputeAlbumSizeOnAlbumChange.php (2)
34-39: Consider using debug-level logging.The logic correctly dispatches the recomputation job when an album is saved. The info-level logging provides visibility into the feature, which is valuable during initial rollout.
However, for long-term production use, consider using
Log::debug()instead ofLog::info()to reduce log volume, as album saves may be frequent operations.🔎 Optional refactor
-Log::info("Album {$event->album->id} saved, dispatching recompute job"); +Log::debug("Album {$event->album->id} saved, dispatching recompute job");
48-59: Consider using debug-level logging.The logic is correct: when an album is deleted, the parent album's statistics are recomputed (if a parent exists). The
dispatchIfproperly ensures the job is only dispatched whenparent_idis not null, avoiding type errors.The redundant null check on lines 56-58 is acceptable since it's only for conditional logging.
As with
handleAlbumSaved, consider usingLog::debug()for long-term production to reduce log volume.🔎 Optional refactor
if ($event->parent_id !== null) { - Log::info("Album deleted from parent {$event->parent_id}, dispatching recompute job"); + Log::debug("Album deleted from parent {$event->parent_id}, dispatching recompute job"); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
app/Actions/Photo/MoveOrDuplicate.phpapp/Actions/Photo/Pipes/Shared/Save.phpapp/Events/PhotoSaved.phpapp/Listeners/RecomputeAlbumSizeOnAlbumChange.phpapp/Listeners/RecomputeAlbumSizeOnPhotoMutation.phpapp/Listeners/RecomputeAlbumStatsOnPhotoChange.phpapp/Models/Album.phpapp/Providers/EventServiceProvider.phplang/el/maintenance.phplang/fr/maintenance.phplang/hu/maintenance.phplang/it/maintenance.phplang/ja/maintenance.phplang/pl/maintenance.phplang/pt/maintenance.phplang/sk/maintenance.phplang/sv/maintenance.phplang/vi/maintenance.phplang/zh_CN/maintenance.phplang/zh_TW/maintenance.phptests/Feature_v2/Statistics/AlbumSpaceTest.phptests/Feature_v2/Statistics/TotalAlbumSpaceTest.phptests/Precomputing/CoverSelection/EventListenersTest.phptests/Precomputing/CoverSelection/EventPropagationIntegrationTest.phptests/Precomputing/SizeComputations/AlbumSizeEventListenerTest.php
🚧 Files skipped from review as they are similar to previous changes (10)
- lang/ja/maintenance.php
- lang/zh_TW/maintenance.php
- lang/sv/maintenance.php
- lang/el/maintenance.php
- lang/it/maintenance.php
- lang/sk/maintenance.php
- tests/Precomputing/SizeComputations/AlbumSizeEventListenerTest.php
- lang/pt/maintenance.php
- app/Providers/EventServiceProvider.php
- app/Models/Album.php
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Any new PHP file should contain the license header and have a single blank line after the opening PHP tag
Variable names should be in snake_case in PHP
Apply the PSR-4 coding standard in PHP
Use in_array() with true as the third parameter in PHP
Only use booleans in if statements, not integers or strings
Use strict comparison (===) instead of loose comparison (==)
Avoid code duplication in both if and else statements
Do not use empty() in PHP
Use the moneyphp/money library for handling monetary values in PHP
Never use floats or doubles to represent monetary values; use integers representing the smallest currency unit (e.g., cents for USD)
**/*.php: Write or extend executable specifications (unit, behaviour, or scenario tests) ahead of implementation, confirm they fail, and then drive code to green before refactoring. List the expected success, validation, and failure branches and add thin failing tests for each path.
For PHP code, adhere to conventions: license headers in new files, strict comparison (===), no empty(), in_array() with third parameter true, snake_case variables, PSR-4 standard, test base classes (AbstractTestCase for Unit, BaseApiWithDataTest for Feature_v2).
Always run phpunit tests. If a test remains red, disable it with a TODO, note the reason, and capture the follow-up in the relevant plan.
Spotless now uses Palantir Java Format 2.78.0 with a 120-character wrap; configure IDE formatters to match before pushing code changes.
Keep each increment's control flow flat by delegating validation/normalisation into tiny pure helpers that return simple enums or result records, then compose them instead of introducing inline branching that inflates the branch count per change.
When introducing new helpers/utilities or editing files prone to style violations (records, DTOs, generated adapters), run the narrowest applicable lint target (for example phpstan) before the full pipeline. Note the command in the related plan/task.
For PHP changes, ru...
Files:
tests/Feature_v2/Statistics/TotalAlbumSpaceTest.phpapp/Actions/Photo/MoveOrDuplicate.phplang/zh_CN/maintenance.phptests/Precomputing/CoverSelection/EventPropagationIntegrationTest.phplang/hu/maintenance.phplang/vi/maintenance.phplang/fr/maintenance.phpapp/Listeners/RecomputeAlbumStatsOnPhotoChange.phpapp/Listeners/RecomputeAlbumSizeOnAlbumChange.phptests/Feature_v2/Statistics/AlbumSpaceTest.phpapp/Actions/Photo/Pipes/Shared/Save.phpapp/Listeners/RecomputeAlbumSizeOnPhotoMutation.phpapp/Events/PhotoSaved.phplang/pl/maintenance.phptests/Precomputing/CoverSelection/EventListenersTest.php
tests/Feature_v2/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests in the tests/Feature_v2 directory should extend from BaseApiWithDataTest
Files:
tests/Feature_v2/Statistics/TotalAlbumSpaceTest.phptests/Feature_v2/Statistics/AlbumSpaceTest.php
tests/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
No need to mock the database in tests; use the in-memory SQLite database instead
Files:
tests/Feature_v2/Statistics/TotalAlbumSpaceTest.phptests/Precomputing/CoverSelection/EventPropagationIntegrationTest.phptests/Feature_v2/Statistics/AlbumSpaceTest.phptests/Precomputing/CoverSelection/EventListenersTest.php
🧠 Learnings (16)
📚 Learning: 2025-12-28T18:12:55.752Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3901
File: app/Providers/AppServiceProvider.php:0-0
Timestamp: 2025-12-28T18:12:55.752Z
Learning: When using Laravel Octane's tick API, Octane::tick(...) returns an InvokeTickCallable that only has ->seconds(int) and ->immediate() methods. There is no ->every(N) method. Use the correct usage: Octane::tick('name', fn() => ...)->seconds(N) or Octane::tick('name', fn() => ..., N). Apply this guideline to PHP files across the project (not just AppServiceProvider.php).
Applied to files:
tests/Feature_v2/Statistics/TotalAlbumSpaceTest.phpapp/Actions/Photo/MoveOrDuplicate.phplang/zh_CN/maintenance.phptests/Precomputing/CoverSelection/EventPropagationIntegrationTest.phplang/hu/maintenance.phplang/vi/maintenance.phplang/fr/maintenance.phpapp/Listeners/RecomputeAlbumStatsOnPhotoChange.phpapp/Listeners/RecomputeAlbumSizeOnAlbumChange.phptests/Feature_v2/Statistics/AlbumSpaceTest.phpapp/Actions/Photo/Pipes/Shared/Save.phpapp/Listeners/RecomputeAlbumSizeOnPhotoMutation.phpapp/Events/PhotoSaved.phplang/pl/maintenance.phptests/Precomputing/CoverSelection/EventListenersTest.php
📚 Learning: 2026-01-03T12:36:02.514Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3922
File: app/Listeners/RecomputeAlbumSizeOnPhotoMutation.php:63-69
Timestamp: 2026-01-03T12:36:02.514Z
Learning: In Lychee, "deleting a photo from an album" (removing the photo-album association) is distinct from "deleting a photo entirely". When PhotoDeleted event is dispatched with a specific album_id, it indicates the photo was removed from that particular album only. The photo may still exist in other albums due to the many-to-many relationship, so only the specified album's size statistics need recomputation.
Applied to files:
app/Actions/Photo/MoveOrDuplicate.phptests/Precomputing/CoverSelection/EventPropagationIntegrationTest.phpapp/Events/PhotoSaved.phptests/Precomputing/CoverSelection/EventListenersTest.php
📚 Learning: 2025-09-16T21:56:01.607Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:65-73
Timestamp: 2025-09-16T21:56:01.607Z
Learning: The validation for album-photo membership in OrderService::addPhotoToOrder() is implemented within PurchasableService::getEffectivePurchasableForPhoto() using a whereHas('albums') constraint that ensures the photo belongs to the specified album_id before applying pricing logic.
Applied to files:
app/Actions/Photo/MoveOrDuplicate.phptests/Precomputing/CoverSelection/EventPropagationIntegrationTest.phpapp/Listeners/RecomputeAlbumStatsOnPhotoChange.phpapp/Listeners/RecomputeAlbumSizeOnPhotoMutation.phptests/Precomputing/CoverSelection/EventListenersTest.php
📚 Learning: 2025-09-18T13:37:32.687Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/PurchasableService.php:286-296
Timestamp: 2025-09-18T13:37:32.687Z
Learning: The deleteMultipleAlbumPurchasables() method in PurchasableService is designed to delete ALL purchasables (both album-level with photo_id IS NULL and photo-level with photo_id IS NOT NULL) for the specified album_ids. This is the intended behavior when albums are being deleted, as all photos within those albums will also be deleted, making their purchasables obsolete.
Applied to files:
app/Actions/Photo/MoveOrDuplicate.phpapp/Listeners/RecomputeAlbumSizeOnPhotoMutation.php
📚 Learning: 2025-12-19T21:01:32.168Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3838
File: lang/pl/webshop.php:1-2
Timestamp: 2025-12-19T21:01:32.168Z
Learning: In the Lychee repository, PHP files under the lang/ directory (and its subdirectories) do not require the standard license header. This is an exception to the general PHP license header rule. Ensure all non-lang PHP files continue to include the license header.
Applied to files:
lang/zh_CN/maintenance.phplang/hu/maintenance.phplang/vi/maintenance.phplang/fr/maintenance.phplang/pl/maintenance.php
📚 Learning: 2025-12-19T21:01:45.910Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3838
File: lang/ar/webshop.php:1-2
Timestamp: 2025-12-19T21:01:45.910Z
Learning: Do not require license headers for language translation files under the lang/ directory (e.g., lang/ar/webshop.php). These resource files are exempt from header checks; apply header enforcement to other PHP source files in the repo.
Applied to files:
lang/zh_CN/maintenance.phplang/hu/maintenance.phplang/vi/maintenance.phplang/fr/maintenance.phplang/pl/maintenance.php
📚 Learning: 2026-01-03T12:36:02.514Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3922
File: app/Listeners/RecomputeAlbumSizeOnPhotoMutation.php:63-69
Timestamp: 2026-01-03T12:36:02.514Z
Learning: In Lychee, treat a PhotoDeleted event with a specific album_id as removing the photo from that album only (not deleting the photo globally). For any listener handling photo mutations, when such an event occurs, recompute the size/statistics for only the affected album, because the photo may remain in other albums due to the many-to-many relationship. Ensure no global deletion logic is triggered and that only the targeted album’s statistics are refreshed.
Applied to files:
app/Listeners/RecomputeAlbumStatsOnPhotoChange.phpapp/Listeners/RecomputeAlbumSizeOnAlbumChange.phpapp/Listeners/RecomputeAlbumSizeOnPhotoMutation.php
📚 Learning: 2025-08-18T10:19:04.946Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3626
File: database/migrations/2025_06_07_144157_photo_tags_to_table.php:87-105
Timestamp: 2025-08-18T10:19:04.946Z
Learning: In the Lychee photo management system, the migration `2025_06_07_144157_photo_tags_to_table.php` runs on data that only contains comma-separated tag names in tag_albums.show_tags - OR/AND expressions do not exist at this migration time, so handling them is unnecessary.
Applied to files:
app/Listeners/RecomputeAlbumStatsOnPhotoChange.phpapp/Listeners/RecomputeAlbumSizeOnPhotoMutation.php
📚 Learning: 2025-09-13T14:47:30.798Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: database/factories/PurchasableFactory.php:103-109
Timestamp: 2025-09-13T14:47:30.798Z
Learning: Photos and albums have a Many-Many relationship in Lychee, meaning a photo can belong to multiple albums. There is no single photo->album_id property. When creating purchasables for photos, both photo_id and album_id must be specified to indicate which photo-album combination the purchasable applies to.
Applied to files:
app/Listeners/RecomputeAlbumStatsOnPhotoChange.phpapp/Listeners/RecomputeAlbumSizeOnPhotoMutation.phptests/Precomputing/CoverSelection/EventListenersTest.php
📚 Learning: 2025-09-16T21:56:01.607Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:65-73
Timestamp: 2025-09-16T21:56:01.607Z
Learning: The validation for album-photo membership in PurchasableService::getEffectivePurchasableForPhoto() uses a JOIN query with the photo_album pivot table to ensure that only purchasables for albums that actually contain the specified photo are returned, preventing price spoofing attacks where clients could use arbitrary album_ids.
Applied to files:
app/Listeners/RecomputeAlbumStatsOnPhotoChange.phpapp/Listeners/RecomputeAlbumSizeOnPhotoMutation.php
📚 Learning: 2025-09-28T12:43:29.852Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3721
File: resources/js/views/gallery-panels/Album.vue:200-204
Timestamp: 2025-09-28T12:43:29.852Z
Learning: The photoStore.load() method in resources/js/stores/PhotoState.ts already handles undefined photoId cases internally with a short circuit exit, so conditional checks before calling photoStore.load() are unnecessary.
Applied to files:
app/Actions/Photo/Pipes/Shared/Save.php
📚 Learning: 2025-09-21T20:09:31.762Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3702
File: database/migrations/2025_08_27_203030_create_purchasable.php:35-39
Timestamp: 2025-09-21T20:09:31.762Z
Learning: In the Lychee codebase, ildyria has decided to ignore the unique constraint issue with UNIQUE(album_id, photo_id) in the purchasables table where NULL values in MySQL/MariaDB don't enforce uniqueness as expected. This is acceptable for their multi-database support strategy (sqlite, pgsql, mariadb) and likely handled at the application level.
Applied to files:
app/Listeners/RecomputeAlbumSizeOnPhotoMutation.php
📚 Learning: 2025-09-18T07:09:17.176Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/PurchasableService.php:150-168
Timestamp: 2025-09-18T07:09:17.176Z
Learning: In PurchasablePhotoRequest, the validation for photo-album membership is implemented at the request validation level rather than in the service layer, using a count comparison to ensure all photos belong to the target album before creating purchasables.
Applied to files:
app/Listeners/RecomputeAlbumSizeOnPhotoMutation.php
📚 Learning: 2025-09-13T14:01:20.039Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:0-0
Timestamp: 2025-09-13T14:01:20.039Z
Learning: In PurchasableService::getEffectivePurchasableForPhoto(), both the photo-specific and album-level SQL queries include ->where('is_active', true), so the returned purchasable is guaranteed to be active and doesn't need additional is_active checks in calling code.
Applied to files:
app/Listeners/RecomputeAlbumSizeOnPhotoMutation.php
📚 Learning: 2025-08-16T14:00:53.808Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3504
File: resources/js/composables/selections/selections.ts:145-147
Timestamp: 2025-08-16T14:00:53.808Z
Learning: In the Lychee codebase, the maintainer ildyria has indicated that bounds checking for array access in selectables.albums.value[idx] within the albumClick function in resources/js/composables/selections/selections.ts is not necessary, suggesting there are adequate safeguards elsewhere in the code or the access pattern is guaranteed to be safe.
Applied to files:
app/Listeners/RecomputeAlbumSizeOnPhotoMutation.php
📚 Learning: 2025-08-22T06:11:18.329Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3641
File: lang/no/settings.php:9-9
Timestamp: 2025-08-22T06:11:18.329Z
Learning: For lang/* translation files in the Lychee project: only review PHP-related issues (syntax, structure, etc.), not translation content, grammar, or language-related nitpicks. The maintainer ildyria has explicitly requested this approach.
Applied to files:
lang/pl/maintenance.php
🧬 Code graph analysis (6)
tests/Precomputing/CoverSelection/EventPropagationIntegrationTest.php (1)
app/Events/PhotoSaved.php (1)
PhotoSaved(14-25)
app/Listeners/RecomputeAlbumSizeOnAlbumChange.php (3)
app/Events/AlbumDeleted.php (1)
AlbumDeleted(14-27)app/Events/AlbumSaved.php (1)
AlbumSaved(15-26)app/Jobs/RecomputeAlbumSizeJob.php (1)
RecomputeAlbumSizeJob(32-194)
app/Actions/Photo/Pipes/Shared/Save.php (4)
app/Events/PhotoSaved.php (1)
PhotoSaved(14-25)app/DTO/PhotoCreate/StandaloneDTO.php (1)
getPhoto(68-71)app/Contracts/PhotoCreate/PhotoDTO.php (1)
getPhoto(17-17)app/DTO/PhotoCreate/DuplicateDTO.php (1)
getPhoto(61-64)
app/Listeners/RecomputeAlbumSizeOnPhotoMutation.php (3)
app/Events/PhotoDeleted.php (1)
PhotoDeleted(14-27)app/Events/PhotoSaved.php (1)
PhotoSaved(14-25)app/Jobs/RecomputeAlbumSizeJob.php (1)
RecomputeAlbumSizeJob(32-194)
app/Events/PhotoSaved.php (3)
app/Events/AlbumDeleted.php (1)
__construct(24-26)app/Events/AlbumSaved.php (1)
__construct(23-25)app/Events/PhotoDeleted.php (1)
__construct(24-26)
tests/Precomputing/CoverSelection/EventListenersTest.php (1)
app/Events/PhotoSaved.php (1)
PhotoSaved(14-25)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 3️⃣ Dockerfile Lint
- GitHub Check: 2️⃣ PHP dist / 8.5 - mariadb
- GitHub Check: 2️⃣ PHP dist / 8.5 - sqlite
🔇 Additional comments (15)
lang/pl/maintenance.php (1)
98-102: LGTM!The new translation entry follows the correct PHP syntax and structure, matching the pattern of other maintenance entries in this file. The array keys and HTML formatting are consistent with existing entries.
lang/fr/maintenance.php (1)
97-101: Translate to French.The text is in English instead of French. This French translation file should contain French text for consistency with the rest of the file (lines 8-76).
🔎 Suggested French translations
'backfill-album-sizes' => [ - 'title' => 'Album Size Statistics', - 'description' => 'Found %d albums without size statistics.<br/><br/>Equivalent to running: php artisan lychee:backfill-album-sizes', - 'button' => 'Compute sizes', + 'title' => 'Statistiques de Taille des Albums', + 'description' => 'Trouvé %d albums sans statistiques de taille.<br/><br/>Équivalent à exécuter : php artisan lychee:backfill-album-sizes', + 'button' => 'Calculer les tailles', ],Note: Several other recent entries (lines 77-96: 'old-orders', 'fulfill-orders', 'fulfill-precompute', 'flush-queue') also contain English text and should be translated to French.
⛔ Skipped due to learnings
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3641 File: lang/no/settings.php:9-9 Timestamp: 2025-08-22T06:11:18.329Z Learning: For lang/* translation files in the Lychee project: only review PHP-related issues (syntax, structure, etc.), not translation content, grammar, or language-related nitpicks. The maintainer ildyria has explicitly requested this approach.Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3637 File: lang/nl/renamer.php:10-94 Timestamp: 2025-08-20T20:35:04.474Z Learning: In Lychee, translation files are initially created with English strings as placeholders, and actual translations are handled through Weblate (a web-based translation management system). This means finding English text in non-English locale files (like lang/nl/, lang/de/, etc.) is expected and part of their translation workflow, not an issue to flag.Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3654 File: lang/es/gallery.php:210-210 Timestamp: 2025-08-27T08:48:45.672Z Learning: The project maintainer ildyria has indicated that language localization consistency is not a priority ("Lang = don't care"), meaning English text in non-English language files is acceptable and should not be flagged as an issue.Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3654 File: lang/cz/gallery.php:210-210 Timestamp: 2025-08-27T08:48:27.520Z Learning: For this Lychee project, the maintainer prefers to keep language strings in English across all locale files rather than translating them to local languages.lang/vi/maintenance.php (1)
98-102: No concerns. The new entry follows the correct format and is consistent with other maintenance actions in the file. The PHP structure is sound and requires no changes.app/Listeners/RecomputeAlbumStatsOnPhotoChange.php (2)
27-44: LGTM! Event payload adaptation is correct.The changes correctly adapt to the new PhotoSaved event payload structure, which now carries a
photo_idstring instead of a Photo object. The query and log message have been appropriately updated to use$event->photo_id.
53-57: LGTM! PhotoDeleted handling is correct.The PhotoDeleted handler correctly recomputes statistics only for the specified album. Based on learnings, this event with a specific album_id indicates removal of the photo from that album only (not global deletion), which is the correct interpretation for the many-to-many photo-album relationship.
lang/zh_CN/maintenance.php (1)
97-101: LGTM! Translation entry is well-structured.The new translation entry for
backfill-album-sizesis consistent with other maintenance entries and includes all necessary fields (title, description with command reference, and button text).tests/Precomputing/CoverSelection/EventPropagationIntegrationTest.php (1)
34-53: LGTM! PhotoSaved dispatch updated correctly.The dispatch call has been correctly updated to pass
$photo->idinstead of the Photo object, aligning with the new PhotoSaved event signature that expects a string photo_id parameter.tests/Precomputing/CoverSelection/EventListenersTest.php (2)
34-58: LGTM! PhotoSaved event instantiation updated correctly.The event instantiation has been correctly updated to pass
$photo->idinstead of the Photo object, consistent with the refactored PhotoSaved event signature.
65-80: LGTM! PhotoSaved event instantiation updated correctly.The event instantiation has been correctly updated to pass
$photo->idinstead of the Photo object, consistent with the refactored PhotoSaved event signature.tests/Feature_v2/Statistics/AlbumSpaceTest.php (1)
59-59: The assertion is correct. The admin user now has 8 albums in the test data. TheBaseApiWithDataTestbase class createsalbum5owned by the admin at line 233. When the admin callsStatistics::albumSpacewithout filtering by a specific album, the endpoint returns all 8 albums:album1,subAlbum1,album2,subAlbum2,album3,album4,subAlbum4, andalbum5(TagAlbum is a separate entity and not included in the regular albums query).app/Listeners/RecomputeAlbumSizeOnPhotoMutation.php (2)
64-70: LGTM! PhotoDeleted handling correctly targets specific album.The implementation correctly handles the
PhotoDeletedevent by dispatching a recomputation job only for the specific album from which the photo was removed. This aligns with the clarified semantics wherePhotoDeletedwith analbum_idindicates removal from that particular album, not global deletion.Based on learnings, the photo may still exist in other albums due to the many-to-many relationship, so recomputing only the affected album's statistics is the correct approach.
41-52: No changes needed. The code correctly usesPhoto::findOrFail()to fetch the photo by ID from the PhotoSaved event. Since PhotoSaved is dispatched immediately after the photo is successfully saved, the photo is guaranteed to exist in the database, making defensive error handling unnecessary. The pattern aligns with the project's approach to event listeners, as seen in handlePhotoDeleted which similarly avoids defensive checks.app/Actions/Photo/Pipes/Shared/Save.php (1)
26-26: LGTM! Dispatch call correctly updated to pass photo ID.The change from
PhotoSaved::dispatch($state->getPhoto())toPhotoSaved::dispatch($state->getPhoto()->id)correctly aligns with the updatedPhotoSavedevent signature that now accepts astring $photo_idinstead of aPhotoobject.Since the photo is saved on line 22, the
idproperty is guaranteed to be available, making this change safe.app/Actions/Photo/MoveOrDuplicate.php (1)
15-15: LGTM!The import is correctly added to support the new AlbumSaved event dispatching.
app/Listeners/RecomputeAlbumSizeOnAlbumChange.php (1)
1-24: LGTM!The file header follows all coding guidelines: license header is present, proper spacing after the opening PHP tag, PSR-4 compliant namespace, and comprehensive documentation explaining the listener's purpose and responsibilities.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
app/Http/Controllers/Admin/Maintenance/BackfillAlbumSizes.php (1)
87-89: Clarify why check() returns -1 in sync mode.Returning
-1in sync mode prevents the maintenance card from being displayed (Vue component line 3 checksdata > 0). Consider adding a comment explaining this design decision, as it's not immediately obvious why sync mode should hide the UI entirely.📝 Suggested comment
public function check(MaintenanceRequest $request): int { + // Return -1 in sync mode to hide the UI card, since sync mode + // processes only 50 albums per API call and requires multiple clicks if (Config::get('queue.default', 'sync') === 'sync') { return -1; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/Actions/Photo/MoveOrDuplicate.phpapp/Http/Controllers/Admin/Maintenance/BackfillAlbumSizes.phpapp/Http/Controllers/Admin/Maintenance/GenSizeVariants.phpapp/Http/Controllers/Admin/Maintenance/MissingFileSizes.phpresources/js/components/maintenance/MaintenanceBackfillAlbumSizes.vuetests/ImageProcessing/Import/ImportFromServerTest.php
🧰 Additional context used
📓 Path-based instructions (5)
**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.vue: Use TypeScript in composition API for Vue3 and use PrimeVue for UI components
Do not use await async calls in Vue3; use .then() instead
Do not use const function = () => {} syntax; use function functionName() {} instead in Vue3
In Vue3 components, the structure should be first, then <script lang="ts">, then <style>Files:
resources/js/components/maintenance/MaintenanceBackfillAlbumSizes.vue**/*.{vue,ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{vue,ts,js}: For Vue3/TypeScript frontend code, follow coding conventions defined in docs/specs/3-reference/coding-conventions.md for style, naming, and testing practices.
For frontend changes, run npm run check to ensure all frontend tests pass before committing.Files:
resources/js/components/maintenance/MaintenanceBackfillAlbumSizes.vue**/*.{vue,ts,js,css}
📄 CodeRabbit inference engine (AGENTS.md)
For frontend changes, run npm run format to apply frontend code formatting with Prettier before committing.
Files:
resources/js/components/maintenance/MaintenanceBackfillAlbumSizes.vue**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Any new PHP file should contain the license header and have a single blank line after the opening PHP tag
Variable names should be in snake_case in PHP
Apply the PSR-4 coding standard in PHP
Use in_array() with true as the third parameter in PHP
Only use booleans in if statements, not integers or strings
Use strict comparison (===) instead of loose comparison (==)
Avoid code duplication in both if and else statements
Do not use empty() in PHP
Use the moneyphp/money library for handling monetary values in PHP
Never use floats or doubles to represent monetary values; use integers representing the smallest currency unit (e.g., cents for USD)
**/*.php: Write or extend executable specifications (unit, behaviour, or scenario tests) ahead of implementation, confirm they fail, and then drive code to green before refactoring. List the expected success, validation, and failure branches and add thin failing tests for each path.
For PHP code, adhere to conventions: license headers in new files, strict comparison (===), no empty(), in_array() with third parameter true, snake_case variables, PSR-4 standard, test base classes (AbstractTestCase for Unit, BaseApiWithDataTest for Feature_v2).
Always run phpunit tests. If a test remains red, disable it with a TODO, note the reason, and capture the follow-up in the relevant plan.
Spotless now uses Palantir Java Format 2.78.0 with a 120-character wrap; configure IDE formatters to match before pushing code changes.
Keep each increment's control flow flat by delegating validation/normalisation into tiny pure helpers that return simple enums or result records, then compose them instead of introducing inline branching that inflates the branch count per change.
When introducing new helpers/utilities or editing files prone to style violations (records, DTOs, generated adapters), run the narrowest applicable lint target (for example phpstan) before the full pipeline. Note the command in the related plan/task.
For PHP changes, ru...Files:
tests/ImageProcessing/Import/ImportFromServerTest.phpapp/Http/Controllers/Admin/Maintenance/GenSizeVariants.phpapp/Http/Controllers/Admin/Maintenance/MissingFileSizes.phpapp/Http/Controllers/Admin/Maintenance/BackfillAlbumSizes.phpapp/Actions/Photo/MoveOrDuplicate.phptests/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
No need to mock the database in tests; use the in-memory SQLite database instead
Files:
tests/ImageProcessing/Import/ImportFromServerTest.php🧠 Learnings (5)
📚 Learning: 2025-08-16T10:57:51.480Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3504 File: resources/js/views/gallery-panels/Albums.vue:11-13 Timestamp: 2025-08-16T10:57:51.480Z Learning: In resources/js/views/gallery-panels/Albums.vue, the SelectDrag component is intentionally configured with :with-scroll="false" even though the #galleryView container has overflow-y-auto. This is working as intended according to the maintainer, who suggested that overflow-y-auto might be removed instead.Applied to files:
resources/js/components/maintenance/MaintenanceBackfillAlbumSizes.vue📚 Learning: 2025-12-28T18:12:55.752Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3901 File: app/Providers/AppServiceProvider.php:0-0 Timestamp: 2025-12-28T18:12:55.752Z Learning: When using Laravel Octane's tick API, Octane::tick(...) returns an InvokeTickCallable that only has ->seconds(int) and ->immediate() methods. There is no ->every(N) method. Use the correct usage: Octane::tick('name', fn() => ...)->seconds(N) or Octane::tick('name', fn() => ..., N). Apply this guideline to PHP files across the project (not just AppServiceProvider.php).Applied to files:
tests/ImageProcessing/Import/ImportFromServerTest.phpapp/Http/Controllers/Admin/Maintenance/GenSizeVariants.phpapp/Http/Controllers/Admin/Maintenance/MissingFileSizes.phpapp/Http/Controllers/Admin/Maintenance/BackfillAlbumSizes.phpapp/Actions/Photo/MoveOrDuplicate.php📚 Learning: 2026-01-03T12:36:02.514Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3922 File: app/Listeners/RecomputeAlbumSizeOnPhotoMutation.php:63-69 Timestamp: 2026-01-03T12:36:02.514Z Learning: In Lychee, "deleting a photo from an album" (removing the photo-album association) is distinct from "deleting a photo entirely". When PhotoDeleted event is dispatched with a specific album_id, it indicates the photo was removed from that particular album only. The photo may still exist in other albums due to the many-to-many relationship, so only the specified album's size statistics need recomputation.Applied to files:
app/Http/Controllers/Admin/Maintenance/GenSizeVariants.phpapp/Actions/Photo/MoveOrDuplicate.php📚 Learning: 2025-09-16T21:56:01.607Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3683 File: app/Actions/Shop/OrderService.php:65-73 Timestamp: 2025-09-16T21:56:01.607Z Learning: The validation for album-photo membership in OrderService::addPhotoToOrder() is implemented within PurchasableService::getEffectivePurchasableForPhoto() using a whereHas('albums') constraint that ensures the photo belongs to the specified album_id before applying pricing logic.Applied to files:
app/Actions/Photo/MoveOrDuplicate.php📚 Learning: 2025-09-18T13:37:32.687Z
Learnt from: ildyria Repo: LycheeOrg/Lychee PR: 3683 File: app/Actions/Shop/PurchasableService.php:286-296 Timestamp: 2025-09-18T13:37:32.687Z Learning: The deleteMultipleAlbumPurchasables() method in PurchasableService is designed to delete ALL purchasables (both album-level with photo_id IS NULL and photo-level with photo_id IS NOT NULL) for the specified album_ids. This is the intended behavior when albums are being deleted, as all photos within those albums will also be deleted, making their purchasables obsolete.Applied to files:
app/Actions/Photo/MoveOrDuplicate.php🧬 Code graph analysis (4)
app/Http/Controllers/Admin/Maintenance/GenSizeVariants.php (1)
app/Events/PhotoSaved.php (1)
PhotoSaved(14-25)app/Http/Controllers/Admin/Maintenance/MissingFileSizes.php (1)
app/Events/PhotoSaved.php (1)
PhotoSaved(14-25)app/Http/Controllers/Admin/Maintenance/BackfillAlbumSizes.php (1)
app/Jobs/RecomputeAlbumSizeJob.php (1)
RecomputeAlbumSizeJob(32-194)app/Actions/Photo/MoveOrDuplicate.php (3)
app/Events/AlbumSaved.php (1)
AlbumSaved(15-26)app/Contracts/Http/Requests/HasFromAlbum.php (1)
from_album(18-18)app/Http/Requests/Traits/HasFromAlbumTrait.php (1)
from_album(20-23)🔇 Additional comments (4)
app/Actions/Photo/MoveOrDuplicate.php (1)
73-73: LGTM!The event dispatching logic correctly passes the
Albuminstance toAlbumSaved::dispatchIf(), and the comment accurately describes this as the destination album where photos were added.app/Http/Controllers/Admin/Maintenance/MissingFileSizes.php (1)
54-54: LGTM!The
PhotoSavedevent dispatch correctly passes the photo ID string to trigger album size recomputation after updating variant file sizes.app/Http/Controllers/Admin/Maintenance/GenSizeVariants.php (1)
66-67: LGTM!The
PhotoSavedevent dispatch correctly triggers album size recomputation after processing each photo's size variants, ensuring statistics stay in sync.resources/js/components/maintenance/MaintenanceBackfillAlbumSizes.vue (1)
27-77: LGTM!The component correctly follows Vue 3 composition API conventions with TypeScript, uses PrimeVue components, avoids async/await in favor of
.then(), and uses function declarations. The error handling and loading states are properly managed.
resources/js/components/maintenance/MaintenanceBackfillAlbumSizes.vue
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is 🚀 New features to boost your workflow:
|
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.