Skip to content

Add typed part metadata to Component#600

Merged
akhilles merged 9 commits intomainfrom
typed-parts
Mar 3, 2026
Merged

Add typed part metadata to Component#600
akhilles merged 9 commits intomainfrom
typed-parts

Conversation

@akhilles
Copy link
Contributor

@akhilles akhilles commented Mar 3, 2026

Note

Medium Risk
Touches core Component construction/mutation and schematic/netlist attribute serialization, so regressions could affect BOM/netlist outputs despite added tests and compatibility guards.

Overview
Adds a new typed Part value (builtin.Part(...)) and a Component(part=...) kwarg to carry structured sourcing metadata (MPN, manufacturer, qualifications) while keeping the existing scalar mpn/manufacturer fields.

Updates schematic/netlist conversion to serialize part as a JSON attribute, support list[Part] in properties["alternatives"], and prevent legacy properties["part"] from overwriting the structured payload; component modifiers can also mutate part/alternatives with part kept in sync when scalar fields change. Includes new unit/integration tests, docs/spec updates, and example/workspace board updates demonstrating the new API.

Written by Cursor Bugbot for commit 8773310. This will update automatically on new commits. Configure here.


Open with Devin

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

// Preserve typed part metadata emitted from `Component(part=...)`.
// Legacy `properties["part"]` must not overwrite the structured JSON payload.
if key == crate::attrs::PART {
continue;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unconditional skip drops legacy properties["part"] values

Medium Severity

The properties["part"] key is unconditionally skipped in the properties loop, but the typed part attribute is only emitted earlier when component.part() returns Some. When a component has no typed part= kwarg but does have a legacy properties = {"part": "some-value"}, that property is silently dropped from the output — the typed path doesn't emit anything, and the properties loop skips it. The guard needs to be conditional on component.part().is_some().

Fix in Cursor Fix in Web

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is intentional

@akhilles akhilles requested a review from LK March 3, 2026 00:54
Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 5 additional findings in Devin Review.

Open in Devin Review

Comment on lines +629 to +631
// Legacy `properties["part"]` must not overwrite the structured JSON payload.
if key == crate::attrs::PART {
continue;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Unconditional skip of properties["part"] drops the property when no typed part is set

When converting a component to schematic attributes, properties["part"] is unconditionally skipped at line 630, even when component.part() returns None. This means a user who writes Component(..., properties={"part": "some-legacy-value"}) without a typed part=... kwarg will have their "part" property silently dropped from the schematic output.

Root Cause

The guard at crates/pcb-zen-core/src/convert.rs:630 checks if key == crate::attrs::PART { continue; } without first verifying that a typed part attribute was actually emitted. Meanwhile, remove_consolidated_component_properties at crates/pcb-zen-core/src/lang/component.rs:391-410 does not remove "part" from the properties map, so the value survives into component.properties() but gets unconditionally skipped during serialization.

The two relevant code paths:

  1. Typed part emitted (lines 601-606): if let Some(part) = component.part() { comp_inst.add_attribute("part", ...) }
  2. Properties loop (lines 627-634): unconditionally skips key == "part"

When component.part() is None, path (1) emits nothing and path (2) skips the legacy property → "part" disappears entirely from the schematic attributes.

Impact: Silent data loss for any existing project that stores metadata in properties["part"] without using the new typed part=... kwarg.

Suggested change
// Legacy `properties["part"]` must not overwrite the structured JSON payload.
if key == crate::attrs::PART {
continue;
// Preserve typed part metadata emitted from `Component(part=...)`.
// Legacy `properties["part"]` must not overwrite the structured JSON payload.
if key == crate::attrs::PART && component.part().is_some() {
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@@ -0,0 +1,553 @@
use std::sync::Arc;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason not to do the snapshot_eval! here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mainly because it doesn't snapshot netlist / post-conversion state. I can add snapshot_netlist_eval!. other option is to use the netlist snapshot test stuff in pcb-zen instead of pcb-zen-core

@github-actions
Copy link

github-actions bot commented Mar 3, 2026

Build Performance

Board Base (median) Head (median) Change
demo/DM0001 119ms ±4 115ms ±2 -3.6%
demo/DM0002 106ms ±2 102ms ±2 -3.9%
demo/DM0003 117ms ±2 116ms ±2 -0.7%
arduino/Nano 80ms ±1.0 85ms ±2 1.06× ±0.02 slower
arduino/UNOQ 173ms ±10 169ms ±9 -2.5%

Measured with hyperfine. Times show median ±stddev.

@akhilles akhilles merged commit 42f83f2 into main Mar 3, 2026
13 of 14 checks passed
@akhilles akhilles deleted the typed-parts branch March 3, 2026 21:39
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