[dotCooCoo]

On main, Clip > Loop... and Cmd+Alt+L loop the selected clip four times at once, with no dialog. The reference guide's "Clip Looping" section (Pro Tools Reference Guide 2020.12, Chapter 42, pages 931 and 932) describes a Clip Looping dialog with Number of Loops; Loop Length, typed in the main timebase, where "the last loop iteration will be truncated" when the length is not a whole number of loops; and Loop Until End of Session or Next Clip: https://web.archive.org/web/20210805202452/https://resources.avid.com/SupportFiles/PT/Pro_Tools_Reference_Guide_2020.12.pdf

This changes what the menu item and the shortcut do: they open the dialog. clip.loop run as a command (MCP, the CLI, engine.execute) still opens nothing.

• Clip Looping dialog: Number of Loops, Loop Length in the main counter's time format (filled with four loops), and Loop Until End of Session or Next Clip. Number of Loops keeps clip.loop's count, which counts the clip and its loops together (4 by default, as before). With Loop Length chosen, OK is grayed out until the field reads as a duration longer than zero.
• clip.loop: the new fill: true loops up to the next clip on the track, or else to the session end (the end of the last clip on any track). length now cuts the last loop short to fit, where it used to round down to whole loops. The short last loop is trimmed with Clip::trim_end_to, which shortens its fades to fit and removes a sync point that falls past the cut. A fill or length that needs more than 1,000 loops is refused with an error and no edit, where length used to stop at 1,000 loops.
• clip.unloop: a shorter last clip counts as part of the loop when it shares the loop, no clip of the loop starts where it ends, and either a whole copy comes before it or it holds the same material as the origin just before it. Remove and Flatten take a short last loop too, and leave both halves of a split clip in place.
• ROADMAP.md gets a progress log line.

The Enable Crossfade option is not part of this pull request. SoundCraft takes the session end to be the end of the last clip on any track, so the last clip in a session has nothing to fill up to and gets no loops.

New tests, each failing without the change:

• loop_length_cuts_the_last_iteration_short (engine): 250 samples of a 100-sample clip make a whole loop and a 50-sample one; 300 make two whole loops.
• a_short_last_iteration_fits_its_fades_and_drops_a_sync_point_past_the_cut (engine).
• a_fill_or_length_past_the_loop_limit_is_refused_without_an_edit (engine): 1,001 loops are refused, and 1,000 still loop.
• unloop_removes_a_short_last_iteration_too (engine), also after an edit to the source note or to the short loop's note: it passes on main, where length never makes a short loop, and fails when Unloop takes only clips of equal length.
• unloop_keeps_both_halves_of_a_split_clip (engine): a split loop copy keeps both halves, and a split loop origin is not a loop. It passes on main and fails when a shared loop alone makes clips of any length one loop.
• loop_fill_stops_at_the_next_clip (engine): the last loop ends where the next clip starts.
• loop_fill_without_a_next_clip_runs_to_the_session_end (engine): the loops reach the end of a clip on another track, and the last clip in the session makes none.
• clip_loop_opens_clip_looping_for_the_selected_clip (ui-egui): the dialog opens only with a clip selected, and an untouched length loops exactly four times even when the counter text rounds.
• loop_length_reads_a_duration_in_every_time_format (ui-egui): each time format reads back the length text that fills the field (format_length), for 2 s and 7.5 s.
• clip_looping_sends_the_chosen_option (ui-egui): each option sends its clip.loop params, and a length of zero or text that is not a time is refused.
• clip_looping_fills_up_to_the_next_clip (ui-egui, through the app): the menu opens the dialog without looping, and the third option with OK fills the gap on the demo's Kick track.

Checks:

• cargo test -p soundcraft-engine --lib cmd::clip_more: 15 passed
• cargo test -p soundcraft-ui-egui --lib --test it loop: 4 + 1 passed
• cargo clippy -p soundcraft-engine -p soundcraft-ui-egui --all-targets -- -D warnings -A unused-mut: no warnings (the allowed unused_mut at crates/engine/src/autosave.rs:189 is on main on Windows)
• cargo fmt --all -- --check: clean
• cargo xtask assets and cargo xtask layers: ok

On Windows 11 (x86-64) with rustc 1.99.1, on top of main at aa74bff with #1017, #998 and #1018 merged in:

• cargo xtask ci passes its 7 steps: 1,282 tests passed, 0 failed, 1 ignored.

git merge-tree reports no conflicts with main at aa74bff. With #1016 (Vari-Fi) the code merges cleanly, and ROADMAP.md conflicts because both change its header line and add a line at the top of its progress log and revision tables.

Summary by CodeRabbit

• New Features
  • Added a clip-looping dialog with options to set a loop count, specify a loop length, or loop to the next clip or end of the session.
  • Final loops are shortened when needed to fit the selected length or endpoint.
• Bug Fixes
  • Looping now skips clips with no positive length and preserves the trimmed length of copied clips.
  • Requests exceeding 1,000 loops now return an error.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: storytold/soundcraft
• Branch: main
• File: .coderabbit.yaml
• Commit: aa74bff
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5553d76d-73ac-44c2-ab21-563358f4ac52

Commits

Reviewing files that changed from the base of the PR and between aa74bff and d5be1e6.

Files selected for processing: 7

• ROADMAP.md
• crates/engine/src/cmd/clip.rs
• crates/engine/src/cmd/clip_more.rs
• crates/ui-egui/src/dialogs.rs
• crates/ui-egui/src/menus.rs
• crates/ui-egui/tests/it/clip_looping.rs
• crates/ui-egui/tests/it/main.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Walkthrough

The change adds a Clip Looping dialog with count, length, and fill options. The engine supports fill targets and shortened final copies, and loop-chain detection handles shortened iterations during unlooping.

Changes: Clip Looping

Layer / File(s)	Summary
Loop targets and copy generation crates/engine/src/cmd/clip.rs	clip.loop supports fill-to-next-clip or session-end targets, trims a final copy to fit, skips nonpositive-length clips, and rejects requests above 1,000 loops.
Shortened iterations and unloop handling crates/engine/src/cmd/clip_more.rs	Loop-chain detection recognizes shortened final iterations. Tests cover trimming, unlooping edited copies, split clips, fill boundaries, and the loop limit.
Dialog, menu wiring, and validation crates/ui-egui/src/dialogs.rs, crates/ui-egui/src/menus.rs, crates/ui-egui/tests/it/*, ROADMAP.md	The menu opens a dialog for count, length, or fill looping. The dialog parses duration inputs and submits clip.loop; tests cover parameter selection and the end-to-end fill flow. The roadmap records the feature.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Menu as Menu
  participant Dialog as Clip Looping dialog
  participant Engine as clip.loop command
  Menu->>Dialog: Open dialog
  Dialog->>Engine: Submit count, length, or fill parameters
  Engine->>Engine: Generate copies and trim final copy when needed

Loading

Possibly related issues

• [Audit] Clip-multiplying edits end in a quadratic hang #915 — The PR adds a 1,000-loop limit to clip.loop, but the available summary does not establish that it resolves the issue’s broader excessive-edit-work problem.

Possibly related PRs

• storytold/soundcraft#960 — This PR calls trim_end_to when trimming a shortened final loop copy.

Suggested reviewers: echelon, claudio911015, grovebit

Merge Risk: Minimal · up to d5be1

The identified looping edge cases do not block merging; the change is ready after normal checks.

Security architecture risk: Low · up to d5be1

The changes remain within the active session and preserve whole-edit rollback. However, the new dialog does not retain its original clip targets, so a selection change before confirmation can redirect the edit.

Retained concerns

• Low · architecture · inferred: The new loop dialog captures duration and start context from the opening selection but retains no clip IDs and submits only count, length, or fill. The engine resolves the selection again when OK executes. A selection changed through another authorized editing path can therefore redirect the pending operation while retaining values prepared for the original clip. This is an edit-ownership risk within the active session; whole-document undo contains its consequences.

Security review details

Security Blast Radius

• inferred — The inspected mutation scope is the active session’s targeted clips and their tracks. Fill reads the session-wide content endpoint but does not independently select other tracks for mutation. Its copy limit is per target, not a command-wide resource budget.

Trust Boundaries and Controls

• observed — Direct app.run calls still execute through Engine::execute rather than opening the dialog. Length retains the existing position parser and bounds, count is clamped to 1–512, and endpoint-driven looping rejects excessive repetitions. These controls constrain edit parameters without establishing authorization for external command transports.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 24.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 6 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly describes the main change: Clip > Loop... opens a Clip Looping dialog with loop-count, loop-length, and fill-until-end options. It is specific and related to the changeset.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

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

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 Clippy (1.99.0)

Clippy execution failed

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