[dotCooCoo]

What and why

Delete Artboards, Delete Empty Artboards and Cut Artboard removed their artboards one at a time. For each artboard, Vec::remove shifted every later artboard, and retain_guides (crates/doc/src/document_guides.rs) collected every ruler guide and walked the layer tree again. Deleting k of n artboards with G guides cost about k × (n + G + the layer tree). The help page describes selecting one or more artboards and deleting them with one command, and Delete Empty Artboards in the Artboards panel menu as removing every empty artboard at once, at Delete artboards.

• remove_artboards (crates/engine/src/cmd/layer.rs) drops all the chosen artboards in one Vec::retain, retires each removed artboard's id, and calls retain_guides once with the set of removed ids. It returns "no such artboard" for an index past the end and "a document needs at least one artboard" when every artboard is chosen, before it changes anything.
• Cut Artboard (crates/engine/src/cmd/panelcmds.rs) and Artwork Cleanup's Empty Artboards (crates/engine/src/cmd/addons/cleanup/checks.rs) call the same helper instead of their own copies of the removal.
• docs/target-app-parity.md has one line for it under Delete Empty Artboards.

The result does not change: the same artboards, guides, selected guides and key guide are left, each command is still one undo step, artboard.deleteEmpty returns the same deleted indices, Delete Artboard and Cut Artboard still refuse locked artboards, Delete Empty Artboards still leaves them, and a document still keeps at least one artboard.

How it was tested

New tests in crates/engine/src/tests/panelcmds.rs, each failing without the change (with the per-artboard loop they count 42 and 58 walks instead of 2):

• deleting_many_artboards_walks_the_artboards_and_guides_once: 40 artboards, each with a guide, plus a guide on no artboard, all selected, with the key guide on a deleted artboard. Deleting 21 of them walks the artboards once and the guides once, leaves the same artboards, guides, selected guides and key guide as deleting them one at a time, takes one undo step, and Undo brings back all 40 artboards and 41 guides.
• delete_empty_and_cut_artboards_walk_the_artboards_and_guides_once: Delete Empty Artboards with 29 of 30 artboards empty, and Cut Artboard of 26 of 30, each with two walks and the same result as deleting one at a time. Cutting all four artboards left is refused and leaves them.

The existing test cmd::addons::cleanup::tests::deleting_many_empty_artboards_walks_the_artboards_and_guides_once now reads the shared helper's walk counter.

Focused checks on the final tree, on Linux. The branch is one commit on main at c9b2119552 and merges cleanly with it. It conflicts with open #7229 in crates/engine/src/cmd.rs.

• cargo fmt --all -- --check
• cargo clippy -p vectorcraft-engine --all-targets -- -D warnings
• cargo check --workspace --tests
• cargo test -p vectorcraft-engine --lib artboard: 248 passed, 0 failed
• cargo test -p vectorcraft-engine --lib cleanup: 35 passed, 0 failed
• cargo xtask brands, layers (30 crates) and assets (337 files)
• cargo xtask ci runs in this pull request's CI.

Checklist

• cargo xtask ci passes (fmt, clippy, tests, assets, brands, layers, wasm).
• No panics in shipped code: no unwrap(), expect(), panic!, unreachable!, todo! or
  unimplemented! outside tests, and no unchecked indexing on data from files, parameters or
  messages. Errors are returned as Result and shown to the user
  (why and how).
• Clean-room: nothing copied from Adobe products or GPL/AGPL code (AGENTS.md).
• Every new asset has a row in ASSETS.md.

Summary by CodeRabbit

• Bug Fixes
  • Improved artboard deletion across direct deletion, empty-artboard cleanup, and cut operations. Related guides are removed consistently, while remaining artboards and guides retain their expected state.
  • Deleting selected artboards now behaves consistently with deleting them individually, including guide selections and undo.
  • Cutting all artboards continues to be refused, and empty deletion requests complete without changing the document.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

Artboard deletion now uses a shared removal function. Empty-artboard cleanup and artboard cutting call this function. Regression tests check deletion results, guide handling, undo behavior, and traversal counts.

Changes: Artboard removal

Layer / File(s)	Summary
Shared removal operation crates/engine/src/cmd.rs, crates/engine/src/cmd/layer.rs	The layer module and remove_artboards are crate-visible. The function validates indices before mutation, rejects deleting every artboard, and returns successfully for an empty list. It removes artboards and associated guides in separate passes, retires deleted IDs, and preserves the remaining artboards’ order and IDs. A test-only counter records the passes.
Deletion command integrations and checks crates/engine/src/cmd/addons/cleanup/checks.rs, crates/engine/src/cmd/addons/cleanup/tests.rs, crates/engine/src/cmd/panelcmds.rs, crates/engine/src/tests/panelcmds.rs, docs/target-app-parity.md	Empty-artboard cleanup and artboard cutting delegate to the shared function. Tests compare bulk and sequential deletion, check guides and guide selections, verify undo behavior and traversal counts, and confirm that cutting every artboard fails without deletion. The parity document describes the shared-pass behavior.

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

Change: Refactor

Suggested reviewers: bflatastic, dylanmtaylor

Merge Risk: Minimal · up to a2efb

Artboard deletion now removes several artboards and their guides in one pass, with tests covering the resulting state and undo. No concrete merge-blocking risk was found.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the main change: deleting several artboards and their guides in one pass. The changeset adds a shared remove_artboards helper that removes chosen artboards in one retain pass and calls…
Description check		The description follows the template sections: What and why, How it was tested, and Checklist. It explains the problem (per-artboard removal and repeated guide and layer walks), the fix (remove_artboa…
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: Title check

Explanation

The title names the main change: deleting several artboards and their guides in one pass. The changeset adds a shared remove_artboards helper that removes chosen artboards in one retain pass and calls retain_guides once. The title matches the changeset and does not need a file list or vague wording.

Full details: Description check

Explanation

The description follows the template sections: What and why, How it was tested, and Checklist. It explains the problem (per-artboard removal and repeated guide and layer walks), the fix (remove_artboards with one retain and one guide pass), affected call sites, and the behavior kept unchanged. It names the new tests with their expected counts and lists the validation commands that were run. The top-level checklist item for cargo xtask ci is unchecked, and the description says CI runs it, which is consistent with the template. The three other checklist items are marked done. Minor gap: the description does not state whether the 'How it was tested' section covers CLI or MCP behavior, which the template asks for.

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.

LanguageTool

docs/target-app-parity.md

LanguageTool checks are incomplete because the per-file request limit of 5 was reached. Remaining text was skipped; findings from completed checks are retained.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: storytold/vectorcraft
• Branch: main
• File: .coderabbit.yaml
• Commit: c9b2119
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: b1987249-14be-4684-b67f-b21c076a605b

Commits

Reviewing files that changed from the base of the PR and between c9b2119 and a2efb2d.

Files selected for processing: 7

• crates/engine/src/cmd.rs
• crates/engine/src/cmd/addons/cleanup/checks.rs
• crates/engine/src/cmd/addons/cleanup/tests.rs
• crates/engine/src/cmd/layer.rs
• crates/engine/src/cmd/panelcmds.rs
• crates/engine/src/tests/panelcmds.rs
• docs/target-app-parity.md

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

[bflatastic]

Merged, thanks @dotCooCoo! It passed its CI checks.