cheapnud/CheapShotcutRandomizer
Watch
1
0
Fork
You've already forked CheapShotcutRandomizer
 0
Code Issues Pull requests Projects Releases 3 Packages Wiki Activity Actions

Add per-cell source assignment for grid compilations #25

Merged
cheapnud merged 1 commit from d/grid-cell-sources into master 2026-08-22 14:49:22 +00:00
Conversation 1 Commits 1 Files changed 4 +210 -22
cheapnud commented 2026-08-22 14:47:46 +00:00
Owner
Copy link
No description provided.
Add per-cell source assignment for grid compilations
All checks were successful
Claude PR Review / AI Code Review (pull_request) Successful in 17s
Details
Build & Test / Build and Test (pull_request) Successful in 40s
Details
2350022737 
Each grid cell can now be fed from a specific source track via a dropdown
per cell; any selected track can fill one or several cells. When cells end
up noticeably different in length, a dialog offers trimming the render to
the shortest cell or keeping the full length.
forgejo-actions commented 2026-08-22 14:48:04 +00:00
Copy link

🤖 Claude AI Code Review

  1. Summary
    Adds per-cell source assignment mode to grid compilations, replacing the boolean split flag with a CellContentMode enum and returning a richer GridCompilationResult, plus a duration-mismatch trim dialog.

  2. Code Quality ⭐⭐⭐⭐☆
    Clean API evolution with good validation guards and test coverage for the new mode and error cases.

  3. Issues

🟡 Home.razor — GetCellAssignment fallback can produce out-of-range cell index. CellLabel/GetCellAssignment are called for cell in [0, _gridLayout), but _gridLayout can be a value other than 2/4 in the loop context. Confirm _gridLayout is always 2 or 4 when AssignedSources is shown; CellNames/BuildCellRects only handle 2 and 4, and CellNames(_gridLayout)[cell] will throw for any other value.

🟢 ShotcutService.cs — cellSources with a source index not present in sourcePlaylists silently falls back to TargetDurationSeconds = 0 via FirstOrDefault(). Not necessarily a bug (0 = default target), but if a cell is assigned a playlist the user never added to sourcePlaylists, the entries may still be pulled correctly by SelectRandomEntries — worth verifying that an unselected sourceIndex is actually valid input. No test covers this path.

🟢 Home.razor — ConfirmCellDurationsAsync compares Entry.Duration sums, but the render trim uses SetRenderRangeToPlaylistDuration on the shortest playlist while the tractor may include a longer track; ensure this matches the same duration definition used elsewhere (per-frame vs seconds). Minor, likely fine given existing usage.

  1. Verdict: APPROVE

Solid change. Just confirm _gridLayout is constrained to 2/4 before the AssignedSources cell loop renders, since CellNames will throw otherwise.

📊 Tokens: 8365 input + 717 output | 💰 Cost: ~$0.0597
Generated by Claude Opus 4.8 via Gitea Actions

## 🤖 Claude AI Code Review 1. **Summary** Adds per-cell source assignment mode to grid compilations, replacing the boolean split flag with a `CellContentMode` enum and returning a richer `GridCompilationResult`, plus a duration-mismatch trim dialog. 2. **Code Quality** ⭐⭐⭐⭐☆ Clean API evolution with good validation guards and test coverage for the new mode and error cases. 3. **Issues** 🟡 Home.razor — `GetCellAssignment` fallback can produce out-of-range cell index. `CellLabel`/`GetCellAssignment` are called for `cell` in `[0, _gridLayout)`, but `_gridLayout` can be a value other than 2/4 in the loop context. Confirm `_gridLayout` is always 2 or 4 when `AssignedSources` is shown; `CellNames`/`BuildCellRects` only handle 2 and 4, and `CellNames(_gridLayout)[cell]` will throw for any other value. 🟢 ShotcutService.cs — `cellSources` with a source index not present in `sourcePlaylists` silently falls back to `TargetDurationSeconds = 0` via `FirstOrDefault()`. Not necessarily a bug (0 = default target), but if a cell is assigned a playlist the user never added to `sourcePlaylists`, the entries may still be pulled correctly by `SelectRandomEntries` — worth verifying that an unselected `sourceIndex` is actually valid input. No test covers this path. 🟢 Home.razor — `ConfirmCellDurationsAsync` compares `Entry.Duration` sums, but the render trim uses `SetRenderRangeToPlaylistDuration` on the shortest playlist while the tractor may include a longer track; ensure this matches the same duration definition used elsewhere (per-frame vs seconds). Minor, likely fine given existing usage. 4. **Verdict**: **APPROVE** Solid change. Just confirm `_gridLayout` is constrained to 2/4 before the `AssignedSources` cell loop renders, since `CellNames` will throw otherwise. --- 📊 **Tokens**: 8365 input + 717 output | 💰 **Cost**: ~$0.0597 *Generated by Claude Opus 4.8 via Gitea Actions*
cheapnud merged commit 3a487f74bf into master 2026-08-22 14:49:22 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
cheapnud/CheapShotcutRandomizer!25
No description provided.