fix(pcb-sch): Keep children of dual-registered ids in hierarchical layout

[detail-app]

Detail bug report: View on Detail

Bug

In crates/pcb-sch/src/hierarchical_layout.rs, two functions made contradictory decisions about an id registered in both component_sizes (a leaf) and module_hierarchy (a module):

• find_root_items treated the id as a module and exiled its children from the root set (they were expected to be laid out when the module was recursed into).
• layout_module_recursive checked component_sizes first, placed the id as a leaf, and returned before recursing into module_hierarchy.

The children were therefore exiled from the roots by one decision and never laid out by the other, so they were silently absent from layout()'s result — no panic, no Result, no log. The defect is an internally inconsistent resolution of an overloaded id, not a missed documented contract (the module doc-comment is silent on dual-registration).

Fix

Two component_sizes.contains_key(module_id) guards, aligning the two readers of module_hierarchy with layout_module_recursive's existing leaf short-circuit:

1. find_root_items — skip exiling a module's children from the root set when that module id is also a registered leaf. The children stay as ordinary roots, matching the leaf interpretation layout_module_recursive already picks, so they are no longer stranded.
2. update_child_positions — return early when the module id is also a leaf. Once the children are reinstated as roots, this function would otherwise re-offset them by the module's packed position, silently corrupting their positions whenever the module packs off-origin.

Both guards are a strict no-op for any well-defined (single-registration) input: component_sizes.contains_key(module_id) is false whenever each id is registered at most once, so the existing layout tests are unaffected. The fix introduces no new panic or Result path and alters behavior only for the self-contradictory dual-registration corner.

Testing

Four regression tests were added to the inline tests module, plus a small layout_entries helper:

• Dual-registration keeps children in the output (both setter orders), asserting the key set is ["C1","M","R1"] (was ["M"]).
• Dual-registration output (keys and positions) equals the leaf-only baseline, for both an at-origin module size and an off-origin one, and both setter orders — the off-origin case is what catches update_child_positions corruption.
• A pure module (no dual registration) still packs its children inside its own bounding box (no-regression guard).
• A dual-registered id nested as the child of a real module does not corrupt its own children's root positions.

Routine checks all pass: cargo nextest run -p pcb-sch (187 passed, 0 skipped, 0 failed — 183 pre-existing + 4 new), doctests, cargo check --workspace --locked, cargo clippy -p pcb-sch --all-targets --all-features -- -D warnings, workspace-wide clippy, and cargo fmt --all -- --check.

End-to-end verification (not versioned): I temporarily added reproducer tests matching the bug report's scenarios, ran them with --nocapture, then removed them so no one-off harness remains in the tree. The dual-registration output is byte-for-byte equal to the leaf-only baseline for both module sizes and both setter orders, and the printed coordinates match the report's expected fixed values exactly (e.g. for M=(4,4) packing off-origin: C1 pos=(0,0) M pos=(13,0) R1 pos=(0,13), identical with and without the redundant add_module). Degenerate inputs (empty input, cycles, self-cycle, diamond) all produce the same output as before the fix — the guards are no-ops since no id is in both maps. The repo's pre-commit hooks (prek run --all-files: ty, cargo fmt, clippy, pcb fmt stdlib) all pass.

A CHANGELOG.md entry was added under Unreleased → Fixed.

---

Automatic Fixes PRs can be configured here.

---

Note

Low Risk
Scoped to schematic hierarchical layout in pcb-sch for a dual-registration edge case; guards are no-ops for typical inputs and are covered by new tests.

Overview
Fixes hierarchical schematic layout when the same id is registered as both a leaf component (set_component_size) and a module (add_module). layout_module_recursive already treated those ids as leaves and never packed module children, but find_root_items still removed those children from the root set, so they disappeared from layout() output with no error.

The PR adds matching component_sizes guards in find_root_items (do not exile children of dual-registered module ids) and update_child_positions (do not re-offset children that were laid out as separate roots). Behavior for normal single-registration modules is unchanged.

Four regression tests plus a layout_entries helper cover stranded children, position parity with a leaf-only baseline (including off-origin packing), pure-module packing, and dual-registered ids nested under a real parent module. CHANGELOG records the fix under 0.4.50.

^{Reviewed by Cursor Bugbot for commit ab1905c. Bugbot is set up for automated code reviews on this repo. Configure here.}

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[cursor]

Stale comment

Not approved: Cursor Bugbot and Cursor Security Agent completed with no findings, but this is a functional hierarchical-layout behavior change rather than a small fixup, so human review is still required. Reviewers were not assigned.

  

_{Sent by Cursor Approval Agent: Pull Request Router and Approver}

[cursor]

Stale comment

Not approved: Cursor Bugbot completed with no findings and Cursor Security Agent was not running, but this is a functional hierarchical-layout behavior change rather than a small fixup, so human review is still required. Reviewers were not assigned.

  

_{Sent by Cursor Approval Agent: Pull Request Router and Approver}

[cursor]

Not approved: Cursor Bugbot completed with no findings and Cursor Security Agent was not running, but this is a functional hierarchical-layout behavior change rather than a small fixup, so human review is still required. Reviewers were not assigned.

  

_{Sent by Cursor Approval Agent: Pull Request Router and Approver}

[detail-app]

Closing because the entire crates/pcb-sch/src/hierarchical_layout.rs module (and the HierarchicalLayout struct, find_root_items, and offset_children functions this PR fixes) was deleted on main by commit c4941f5 (PR #1347, "Trim CI and test cruft"), which removed the unused pcb-sch hierarchical_layout module. Nothing on main references the module, so the bug this PR targets no longer exists in the codebase.