[dotCooCoo]

In Edit Points, right-clicking a segment offered only Straight Segment and Curved Segment (#833 listed Add Point and Delete Segment as still missing). In PowerPoint the segment menu also has Add Point, Delete Segment, Open Path and Close Path. "Add Point option adds a new vertex at the cursor position on the line segment", Delete Segment "is grayed out for Open Paths" and "removes the segment entirely", and Open Path opens a closed path "at the vertex which is located just before this selected line segment" (Indezine, Edit Points in Motion Paths in PowerPoint 2016; Add Point on a shape's outline: Add or Delete Points for Shapes in PowerPoint 2016). Microsoft's Draw or edit a freeform shape page covers only Ctrl-click for adding and deleting points.

What changes:

• The segment menu has Add Point, Delete Segment and Open Path or Close Path after Straight/Curved Segment. Add Point puts the point where the right click was: context_click keeps the place on the segment that segment_at finds (ToolState::edit_segment_t).
• segment_at gave that place only in steps of 1/32 of the segment, so a point added with Ctrl-click (tools.rs:617-620) could land away from the pointer: at x=210.938 for a click at x=210 on an edge 150 pt long. It now narrows the place down after the first pass.
• shape.deleteSegment {path?, segment, id?} removes a segment of a closed outline, so the outline opens there. Where PowerPoint's documentation does not say how the points are ordered afterwards, DeckCraft starts the outline at the segment's end and ends it at its start. On an open outline the command returns an error and the menu item is grayed out. Each deletion is one undo step.
• Open Path on a segment opens the outline at the segment's start point.
• shape.addPoint and shape.deletePoint now forget a point type remembered for handle drags, as shape.openPath and shape.closePath already do.
• shape.addPoint on the first point of an open outline returned no error when a closed outline followed it in the same path: it split that outline's closing edge. It now uses ending_segment, which stays within the outline.
• docs/gaps.md, the Edit points row of docs/ui-parity.md and the ROADMAP.md progress log list the new items.

Tests:

• tests::edit_points_delete_segment_opens_the_outline: deletes a rectangle's right edge and its closing edge, checks the point order, the error on an open outline and the undo step. It fails on main, where shape.deleteSegment does not exist.
• tests::edit_points_right_click_records_the_place_on_the_segment: right clicks a quarter and 40% along an edge and on an ellipse's curve, then Add Point at each place. It does not compile on main (no edit_segment_t); without the narrowing in segment_at it fails with the point at x=210.938 instead of 210.
• tests::edit_points_add_and_delete_point_forget_the_point_type: fails on main after shape.addPoint, and after shape.deletePoint once that one is fixed.
• tests::edit_points_add_point_at_the_start_of_an_open_outline: a donut with its outer ring opened. It fails on main, where Add Point splits the inner ring.
• canvas::tests::edit_points_segment_menu_adds_points_and_deletes_segments (kittest): Add Point, Open Path, Delete Segment, the grayed-out Delete Segment and Close Path from the menu.

Checks: cargo test -p deckcraft-engine --lib (410 passed, 1 ignored), cargo test -p deckcraft-ui-egui --lib canvas -- --test-threads=1 (27 passed), cargo fmt --all -- --check, cargo clippy -p deckcraft-engine -p deckcraft-ui-egui --all-targets -- -D warnings, cargo xtask assets and cargo xtask layers.

On Windows 11 (x86-64) with rustc 1.99.1, on top of main at 38d2ade, with #1292, #1298 and #1300 merged in:

• cargo xtask ci passes its 8 steps: 1,885 tests passed, 0 failed, 6 ignored.

#1308 changes ToolState::vertex_kind next to the new edit_segment_t field in crates/engine/src/tools.rs, and the same row of docs/ui-parity.md. Whichever of the two merges second needs both conflicts resolved: the tools.rs lines keep both sides, and the row is merged by hand.

git merge-tree reports no conflicts with main at 38d2ade.

Summary by CodeRabbit

• New Features
  • The Edit Points right-click menu now lets you add a point at the clicked location on a segment.
  • You can delete a segment from a closed outline, which opens the path at that segment. Delete Segment is unavailable for open outlines.
  • The menu also lets you open or close a path.
• Documentation
  • Updated the Edit Points guides to reflect the available menu actions.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: storytold/deckcraft
• Branch: main
• File: .coderabbit.yaml
• Commit: 38d2ade
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: ee45a499-a321-484e-8530-020dc7392bc3

Commits

Reviewing files that changed from the base of the PR and between 38d2ade and 92ce587.

Files selected for processing: 7

• ROADMAP.md
• crates/engine/src/cmd/points.rs
• crates/engine/src/tests.rs
• crates/engine/src/tools.rs
• crates/ui-egui/src/canvas.rs
• docs/gaps.md
• docs/ui-parity.md

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

Walkthrough

Edit Points now supports adding a point at the clicked segment location, deleting a segment from a closed outline, and opening or closing a path. The engine retains the click position for point insertion and validates segment operations.

Changes: Edit Points segment actions

Layer / File(s)	Summary
Segment and point commands crates/engine/src/cmd/points.rs, crates/engine/src/tests.rs	The engine adds shape.deleteSegment, which opens a closed outline at the selected segment and reports errors when deletion is unavailable. Point insertion resolves the segment ending at a selected vertex, including a closed outline’s closing edge. Segment hit-testing refines its sampled position. Tests cover insertion, deletion, undo, error cases, and clearing remembered point types.
Retain the clicked segment position crates/engine/src/tools.rs	ToolState stores the segment parameter returned by segment_at during a context click, along with the selected path and segment.
Canvas menu actions and documentation crates/ui-egui/src/canvas.rs, ROADMAP.md, docs/gaps.md, docs/ui-parity.md	The segment menu offers Add Point, Delete Segment, and Open/Close Path. Add Point uses the stored click position or defaults to 0.5. Delete Segment is disabled for open paths. Tests cover the menu actions, and the documentation records the available actions.

Priority: Normal

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

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant Canvas
  participant ToolState
  participant PointsCommands
  User->>Canvas: Right-click a segment
  Canvas->>ToolState: context_click
  ToolState->>PointsCommands: segment_at
  PointsCommands-->>ToolState: path, segment, and t
  ToolState->>ToolState: Store edit_segment and edit_segment_t
  User->>Canvas: Select a segment menu action
  Canvas->>PointsCommands: Invoke the selected shape command

Loading

Possibly related issues

• Direct Selection: segment selection should display endpoint anchors as filled/selected, and deleting segments should split path into separate objects vectorcraft#3380: This PR adds segment deletion that opens the outline, but does not implement the issue’s remaining Direct Selection path-structure behavior.
• [BUG] Adding an anchor point to an existing Bézier curve does not behave as expected photocraft#8653: This PR adds insertion on an existing segment and refines hit-testing; the issue’s smooth-point behavior may remain unresolved.

Possibly related PRs

• storytold/deckcraft#1260: This PR extends the segment state, commands, and context menu introduced by that PR.

Suggested reviewers: nikitkara, marcoarnulfo

Merge Risk: Minimal · up to 92ce5

The reviewed segment actions have no established issue requiring a fix before merge.

Security architecture risk: Low · up to 92ce5

The new editing actions retain existing document, shape-lock, validation, and undo checks. No introduced security defect was identified. Deployment-specific access controls were not verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The inspected segment-edit operations affect an eligible shape in the caller's active document. An explicit shape identifier does not bypass active-document resolution or shape-lock validation; no broader document authority was identified in these handlers.

Trust Boundaries and Controls

• observed — The existing headless automation backend forwards caller-selected commands and pointer events to Session. That backend is unchanged against the supplied PR base, so the added command inherits an existing automation route rather than introducing a new transport. This establishes reachability, not deployment-level caller authorization.

Resilience and Maintainability Implications

• observed — Segment actions clear the transient segment selection. Although the click-parameter scalar can remain after selection cleanup, the inspected consumer reads it only when a segment selection exists, and subsequent segment picking replaces both values.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 4 files. (3 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 names the main change: right-click segment context menu actions in Edit Points (Add Point, Delete Segment, Open/Close Path). The raw summary confirms these additions across points.rs, tools.…
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: right-click segment context menu actions in Edit Points (Add Point, Delete Segment, Open/Close Path). The raw summary confirms these additions across points.rs, tools.rs, canvas.rs, tests, and docs, and the PR objectives describe the same menu actions. The title omits the supporting fixes (segment hit-testing refinement, clearing remembered point types, open-outline closing edge fix), which is acceptable because the title need not cover every detail.

Full details: Docstring Coverage

Explanation

Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 4 files. (3 skipped: 3 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

Usage-based review receipt

• Mode: Continue automatically
• Reviewed files: 7
• Waived: $1.75 (charged $0.00)
• View usage details

Note

This review exceeded your plan’s limits and used usage-based reviews—free during trial, billed after paid activation unless disabled. Manage usage-based reviews.

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

[dotCooCoo]

The gate failure is in control_server::tests::a_line_that_is_not_a_request_closes_the_connection, which this pull request does not touch. It also fails on main: the push run for 38d2ade stopped on the same control server tests (run 40094768975). When both connection slots are still held, the server refuses the next connection as busy and drops the socket, so the test's second write sees a reset. #1298 fixes this. On Linux, with each control server test in its own process and several at once, main failed 2 of 660 runs and main with #1298 merged failed 0 of 840.

[dotCooCoo]

cargo xtask ci passes its 8 steps on 92ce587 with #1298, #1292 and #1300 merged in: 1,885 tests passed, 0 failed, 6 ignored. #1298 fixes the control server test that stopped this pull request's CI run. #1292 and #1300 fix two UI tests that also fail on main (38d2ade) on this setup. Without #1292 and #1300, the run stopped at status::tests::a_long_language_name_leaves_the_notes_button_clickable, which #1300 fixes.