Skip to content

MCP: path, type, panel, library, style and ruler indices refuse a wrong-type value - #7402

Merged
bflatastic merged 4 commits into
storytold:mainfrom
dotCooCoo:fix/more-int-param-wrong-type
Nov 14, 2026
Merged

bflatastic merged 4 commits into
storytold:mainfrom
dotCooCoo:fix/more-int-param-wrong-type

Conversation

@dotCooCoo

Copy link
Copy Markdown
Contributor

What and why

Several commands read an index with .and_then(Value::as_u64), so a value such as "3", 2.5, -1 or true was treated as if it had been left out. A required index was reported as missing, and an optional one silently took its default. VectorCraft's own rule is that a supplied value of the wrong type is refused, and a default applies only to a value that is left out.

  • A new helper, index_opt (crates/engine/src/cmd.rs), reads an optional index: left out or null gives None, and any other value that is not a whole number of 0 or more is refused with "{key} must be a whole number", the message object_param and MCP: an integer given as a string or a fraction is refused as the wrong type, not reported as missing #7376 use. The catalogs already hold this message.
  • Required indices now refuse such a value instead of reporting it as missing: anchor of path.setHandle, path.moveAnchor and text.setPathHandle, segment of path.insertAnchor, and to of artboard.reorder, library.moveGroup and graphicStyle.move. Left out or null, each is still reported as missing.
  • The optional subpath of path.appendAnchor, path.close and path.reverse refuses such a value. Before, the command extended or closed the default subpath, and path.reverse reversed every subpath. The read also no longer casts with as usize.
  • The optional artboard of document.rulerOrigin, document.setRulerOrigin and document.resetRulerOrigin refuses such a value with the whole-number message instead of "index out of range". A whole number past the artboards is still "index out of range".
  • This changes one existing behavior: a null artboard on the ruler origin commands is now artboard 0, as a left-out one is, where it used to answer "index out of range".
  • docs/mcp.md and docs/target-app-parity.md describe the new checks.

This does not depend on #7376; the two add separate helpers in cmd.rs and git merge-tree reports no conflict between them. The branch merges cleanly with main at 1e4e3534f5e.

How it was tested

New test tests::params::a_malformed_index_is_not_missing_or_its_default (crates/engine/src/tests/params.rs): for each command above, "3", 2.5, -1, true, [] and {} are refused with the whole-number message naming the key; a required index left out or null is still missing; a whole-number artboard past the artboards is still out of range; the document and undo history are unchanged; and an optional index left out or null still runs with its default. It fails without the change, both with index_opt reading a malformed value as left out (path.setHandle answers "missing anchor") and with only the old path.reverse read restored (it reverses the whole path).

On main at 1e4e3534f5e merged with this branch:

  • cargo fmt --all -- --check: the only diff is in crates/svg/src/lib.rs (line 491), which this branch does not change; main at 1e4e3534f5e has the same diff.
  • cargo clippy -p vectorcraft-engine --all-targets -- -D warnings
  • cargo check --workspace --tests
  • cargo test -p vectorcraft-engine --lib -- tests::library tests::stylepanel tests::panelcmds tests::params tests::textedit tests::draw2 tests::menucmds::artboard: 466 passed, 0 failed.
  • cargo test -p vectorcraft-engine --test it -- ruler_origin: 10 passed, 0 failed.
  • cargo xtask brands, layers and assets.
  • 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.

…ng-type value

A required index given as something other than a whole number of 0 or
more, such as "3", 2.5, -1 or true, was reported as missing: `anchor` of
path.setHandle, path.moveAnchor and text.setPathHandle, `segment` of
path.insertAnchor, and `to` of artboard.reorder, library.moveGroup and
graphicStyle.move. Each now refuses it with "`anchor` must be a whole
number", naming the key; left out or null, it is still missing.

An optional `subpath` of path.appendAnchor, path.close and path.reverse
given that way was read as left out, so the command extended, closed or
reversed the default subpath (path.reverse reversed every subpath). The
`artboard` of the ruler origin commands answered "index out of range"
for such a value and for null. Both now refuse such a value with the
same message and change nothing. A null `artboard` is artboard 0, as one
left out is; a whole number past the artboards is still out of range.
@coderabbitai

coderabbitai Bot commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 7 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

Commits

Reviewing files that changed from the base of the PR and between e56d1c7 and 5aeb42c.

Files selected for processing: 10

  • crates/engine/src/cmd.rs
  • crates/engine/src/cmd/edit.rs
  • crates/engine/src/cmd/library.rs
  • crates/engine/src/cmd/panelcmds.rs
  • crates/engine/src/cmd/path.rs
  • crates/engine/src/cmd/style.rs
  • crates/engine/src/cmd/typecmd.rs
  • crates/engine/src/tests/params.rs
  • docs/mcp.md
  • docs/target-app-parity.md
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@bflatastic
bflatastic merged commit edb228a into storytold:main Nov 14, 2026
7 checks passed
@bflatastic

Copy link
Copy Markdown
Member

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

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants