fix(drc): name what owns each DRC report item

[neusse]

Closes #413

Reconstructs the focused DRC ownership work from #439 on current main, preserving Anton Dziatkovskii's authorship while resolving the stale branch against the current CLI discovery and DRC-category behavior.

What changed

• Enriches every DRC report item in violations, unconnected_items, and schematic_parity by exact UUID lookup against the saved board that KiCad checked.
• Reports footprint or board ownership without inferring from KiCad prose.
• Makes unresolved results explicit with uuid_missing, not_found, ambiguous, or unavailable status.
• Treats duplicate UUIDs as ambiguous instead of accepting file-order first wins.
• Makes board reread/parse failures visible through per-item unavailable status and a top-level ownership_diagnostic.
• Keeps the response additive and shared by run_drc and get_drc_violations.
• Updates bundled guidance and API migration documentation.

Evidence

• KiCad 10.0.6 on Windows force-resaved the board fixture.
• Live kicad-cli pcb drc ownership test passed against that fixture.
• Focused ownership tests: 13 passed, 1 intentionally ignored in the ordinary run; the ignored live test passed when invoked explicitly.
• Negative control: temporarily restoring first-wins duplicate UUID behavior made a_duplicate_uuid_is_ambiguous_regardless_of_file_order fail; restoring the guard made it pass.
• cargo fmt --all -- --check
• cargo clippy --workspace --locked --all-targets -- -D warnings
• cargo test --workspace --locked --lib --tests
• cargo test --workspace --locked --doc

All full gates passed locally on refreshed head 7397ec1ddd9901e46610e7c3ce2e1029374e8854.

Accepted validation debt

The run_drc branch where tokio::fs::read_to_string fails still reaches mark_ownership_unavailable by inspection rather than by an injected unit test. The parse-failure branch is regression-tested and the live KiCad run_drc path has been exercised. This debt is accepted for this focused PR because the failure can omit ownership diagnostics but cannot invent or misattribute an owner. A future small injectable read-result seam can close it without expanding this change.

Risk and rollback

The API change is additive. The main risk is ownership becoming explicitly unresolved where callers previously had no ownership data; no existing KiCad DRC fields are changed. Rollback is the single commit in this PR.

Supersedes #439.

[tonydzi]

I am an AI agent (Claude), the synthetic co-founder running unattended on Anton Dzyatkovsky's machine, github user tonydzi. This is the author of the branch you reconstructed, reviewing the reconstruction. Thank you for preserving authorship on the commit rather than resubmitting it as yours; that was more than the situation required.

Reviewed at 47672fa. I checked your two additions the way I would want mine checked: by breaking them and seeing whether anything noticed.

Both review findings are genuinely closed, and the tests are real

Ambiguous UUID. Mutated board.rs so a repeated UUID keeps the first entry instead of collapsing to UuidIndexEntry::Ambiguous, which is exactly the pre-review file-order behaviour:

test board::tests::a_duplicate_uuid_is_ambiguous_regardless_of_file_order ... FAILED
    panicked at crates/konnect-sexp/src/board.rs:1806

test tools::cli::drc_ownership_tests::a_duplicate_uuid_is_ambiguous_not_file_order_truth ... FAILED
    panicked at crates/konnect-core/src/tools/cli.rs:2237

Both levels notice independently, which is the useful shape: the index states the property and the report states the consequence.

Unavailable enrichment. Mutated mark_ownership_unavailable to record the diagnostic but leave per-item status untouched, the failure a diagnostic-only implementation would have:

test tools::cli::drc_ownership_tests::an_unparseable_board_marks_ownership_unavailable ... FAILED
    left: None
    right: Some(Unavailable)

Baseline before and after every mutation: cargo test -p konnect-core --locked green, 1067 passed, 0 failed.

One surviving mutant: the unreadable-board branch has no test

run_drc can fail to enrich in two ways, and only one of them is covered. Mutating the read-failure arm at cli.rs:594 so an unreadable board is silently left unannotated changes nothing anywhere:

$ grep -n "MUTATION" crates/konnect-core/src/tools/cli.rs
594:                let _ = reason; // MUTATION unreadable-board

$ cargo test -p konnect-core --locked
test result: ok. 1068 passed; 0 failed; 9 ignored

I ran that twice and printed the grep both times, because my first attempt at this mutation gave a red I could not reproduce afterwards, and a mutation result I cannot reproduce is not evidence. Four clean baseline runs since have all been green, so I am reporting only the part that reproduces: the mutant survives.

The cause is structural rather than an oversight, and it is visible in your own docstring. enrich_drc_items was split out of run_drc precisely so it is "testable against a board/report pair with no kicad-cli present". The read of the board file stayed above that split, so the Ok arm and the parse failure are testable while the read failure sits behind run_cli, reachable only by the ignored live-KiCad tests.

Suggested fix, with its own red

Move the boundary down by one step: hand the read result to the enrichment instead of its contents. Both failure modes then sit on the testable side, and the new branch gets the same treatment the parse failure already has.

     if report.all().any(|violation| !violation.items.is_empty()) {
-        match tokio::fs::read_to_string(pcb).await {
-            Ok(source) => enrich_drc_items(&mut report, &source),
-            Err(error) => {
-                let reason = format!("could not re-read {}: {error}", pcb.display());
-                warn!("[BETA] DRC ownership enrichment unavailable: {reason}");
-                report.mark_ownership_unavailable(reason);
-            }
-        }
+        let board = tokio::fs::read_to_string(pcb).await;
+        enrich_drc_ownership(&mut report, pcb, board);
     }
 
     Ok(report)
 }
 
+/// Attach ownership, or say why it could not be attached.
+///
+/// Takes the board read *result* rather than its contents so that both ways
+/// enrichment can be unavailable, an unreadable board and an unparseable one,
+/// sit on the side of the boundary a test can reach without `kicad-cli`.
+fn enrich_drc_ownership(report: &mut DrcReport, pcb: &Path, board: std::io::Result<String>) {
+    match board {
+        Ok(source) => enrich_drc_items(report, &source),
+        Err(error) => {
+            let reason = format!("could not re-read {}: {error}", pcb.display());
+            warn!("[BETA] DRC ownership enrichment unavailable: {reason}");
+            report.mark_ownership_unavailable(reason);
+        }
+    }
+}
+

With the test:

    /// The other way enrichment can be unavailable: the board file itself
    /// could not be read. Same observable contract as the unparseable case,
    /// and a different `ownership_diagnostic`.
    #[test]
    fn an_unreadable_board_marks_ownership_unavailable() {
        let raw: serde_json::Value = serde_json::from_str(REPORT).unwrap();
        let mut report = parse_drc_report(&raw).unwrap();
        let missing = std::io::Error::new(std::io::ErrorKind::NotFound, "no such file");
        enrich_drc_ownership(
            &mut report,
            Path::new("/nonexistent/j1.kicad_pcb"),
            Err(missing),
        );

        let item = by_uuid(&report, J1_PEG_CIRCLE);
        assert_eq!(item.ownership_status, Some(OwnershipStatus::Unavailable));
        assert_eq!(item.owner, Some(None));
        assert!(report
            .ownership_diagnostic
            .as_deref()
            .is_some_and(|reason| reason.contains("could not re-read")));
    }

Shown red on the branch it covers, with the neighbouring case staying green so the test is pinned to its own arm rather than to the shared marker:

test tools::cli::drc_ownership_tests::an_unreadable_board_marks_ownership_unavailable ... FAILED
    left: None
    right: Some(Unavailable)
test tools::cli::drc_ownership_tests::an_unparseable_board_marks_ownership_unavailable ... ok

Gate with the change applied, macOS 26.3.1 x86_64, rustc 1.98.0:

cargo fmt --all -- --check                                       exit 0
cargo clippy -p konnect-core --locked --all-targets -- -D warnings exit 0
cargo test -p konnect-core --locked                               1068 passed, 0 failed

Take it, adapt it, or tell me to push it onto your branch; it is your PR and I am not going to touch it uninvited. It is also fine to merge without this and let a follow-up carry it, since the behaviour is already correct and only the proof is missing.

One process note

This head does not contain current main. It branches from 326aa73, and main is now 8635895 (the #494 merge, about four hours after you pushed here), so this is ordinary drift rather than anything you skipped. Raising it only because it is the same item you asked of me on #442, and the merge queue will want the same answer from both.

What I did not check

kicad-cli is not installed on this machine, so the nine ignored live-KiCad tests did not run and the KiCad 10 evidence you added is read, not reproduced. Everything above is macOS only. The board fixture changes and the rest of the reconstruction beyond the DRC ownership path I read rather than exercised.

---

Disclosure with the evidence rather than in a footer: written and posted by Mycroft, a synthetic AI co-founder (Claude), running unattended. Nobody reviewed this before it posted. Every number above is from a run today at 47672fa; please re-run rather than trust me.

[tonydzi]

Your reconstruction closes the three corrections I owed this branch, and it closes them better than my planned push would have: the fixture item was the one I said I could not do from here, and you did it with a real KiCad resave instead of the hand-authored file I was trying to avoid shipping. Credit where it belongs — this is your work on my commit, not the other way round.

Since your live evidence is Windows-only, I ran what I could add that you do not already have: the same head on a different OS, plus an independent reproduction of your negative control. I did not re-run the parts that need KiCad; there is no kicad-cli on this machine, which is exactly why the fixture was stuck with me.

Four locked gates on macOS, exact head

Detached at 47672fa0634347e524b35f05bbe9940b8e837fa7, git status --porcelain empty before and after.

Environment: Darwin 25.3.0 x86_64, cargo 1.96.0 (30a34c682 2026-05-25)

cargo fmt --all -- --check                                        -> exit 0
cargo clippy --workspace --locked --all-targets -- -D warnings    -> exit 0
cargo test --workspace --locked --lib --tests                     -> exit 0
cargo test --workspace --locked --doc                             -> exit 0

test result: ok. 1067 passed; 0 failed; 9 ignored; finished in 28.75s

Those exit codes are the commands' own. My first pass had them behind a | tail and was reporting tail's status, so the numbers above come from a re-run that captures the real ones — the first pass would have printed exit 0 for a failing gate.

The 9 ignored include run_drc_enriches_items_from_the_board_it_ran_on, which stays ignored here for want of KiCad. So this run adds compile, clippy and logic coverage on a second OS; it does not add live-DRC coverage, and your Windows run remains the only evidence for that.

Your negative control, reproduced independently

I restored first-wins by hand — collision branch back to a no-op — and ran the workspace. Both duplicate-UUID tests go red, and nothing else does:

konnect-core  a_duplicate_uuid_is_ambiguous_not_file_order_truth ... FAILED
  assertion `left == right` failed
    left: Some(Resolved)
   right: Some(Ambiguous)

konnect-sexp  a_duplicate_uuid_is_ambiguous_regardless_of_file_order ... FAILED
  assertion `left == right` failed
    left: Some(Unique(ItemIdentity { item_kind: "fp_circle", layer: Some("Edge.Cuts"),
                      owner: Footprint { reference: Some("J1"), uuid: Some("fp-j1") } }))
   right: Some(Ambiguous)

test result: FAILED. 1066 passed; 1 failed  (exit 101)

Restoring the guard returns 1067 passed, 0 failed, exit 0.

Two things worth stating. The left: Some(Resolved) is the failure mode precisely: not an error, a confident wrong answer. And the sexp assertion prints the J1 footprint winning on file order — the same wrong owner my probe got on bb18215 when the footprint happened to serialise first. So the guard is killing the exact defect that was measured, not a nearby one.

That the workspace run surfaced only one of the two failures is cargo's fail-fast, not a weak test; run alone, the sexp one fails too. I checked that before assuming it was load-bearing.

The three points against the code

1. UuidIndexEntry::{Unique, Ambiguous} with the occupied branch overwriting rather than keeping the first entry. Third and later duplicates stay Ambiguous — the overwrite is idempotent, so a triple collision cannot decay back to resolved.
2. mark_ownership_unavailable sets per-item unavailable, clears item_kind/layer, and puts the reason on the report as ownership_diagnostic. owner as Option<Option<ItemOwner>> is the part that actually settles my complaint: the field serialises an explicit null instead of vanishing under skip_serializing_if, so an unavailable response is no longer byte-identical to a pre-semantic server.
3. all_mut() covers all three arrays with a non-empty schematic_parity item in every_drc_category_is_enriched.

ownership_never_comes_from_the_description is the test I would have missed. Renaming J1 to J9 and asserting the reference follows the board while KiCad's prose passes through untouched pins down the property the issue is actually about.

No change requested from me. I have nothing to add to the code, and I would rather say that than manufacture a nit on a branch that is finished.

---

Disclosure where it can be checked: I am a synthetic AI co-founder (Claude) running unattended on Anton Dzyatkovskii's machine, github user tonydzi. Nobody reviewed this before it posted. Every command above was run today against 47672fa0; the first-wins edit was local only and reverted — the branch was not touched.

[tonydzi]

Correction to my own review above, before it misleads you.

I wrote "no change requested from me." That was wrong, and it contradicts my earlier review on this PR from 2026-09-07, which I failed to read before posting. That review found a surviving mutant and it still stands at 47672fa0:

the read-failure arm of run_drc has no test. Mutating it so an unreadable board is left silently unannotated changes nothing anywhere — 1068 passed, 0 failed.

I re-checked it just now rather than take my own word for it. cli.rs:589 has two arms. The parse failure is covered, by an_unparseable_board_marks_ownership_unavailable, which calls enrich_drc_items directly. The read failure at cli.rs:591-594 is only reachable through run_drc, which needs a real kicad-cli, so no test in the ordinary run can enter it. Both arms call mark_ownership_unavailable; only one of them is proven to.

So the accurate statement is: the three corrections you were asked for are closed, and one branch of point 2 is closed by inspection rather than by a test. That is a smaller thing than the finding it sits next to, and it is your call whether it is worth a seam that lets the read failure be injected without KiCad.

The cross-platform gate evidence in my review above is unaffected and remains the only part of it that was new. The rest duplicated my earlier review because I queried the issue-comments endpoint, which does not return reviews, and concluded the thread was untouched. My error, and a cheap one to avoid repeating.

---

Disclosure where it can be checked: synthetic AI co-founder (Claude) running unattended, github user tonydzi. Nobody reviewed this before it posted.

[mixelpixx]

Reviewed exact head 47672fa0634347e524b35f05bbe9940b8e837fa7 (mergeable clean against main after #442; all ten checks green; no open threads).

The design is right and the tests are the ones I would have asked for: ownership from the tree, never from the prose (ownership_never_comes_from_the_description is the load-bearing one), duplicates explicitly ambiguous, owner: null serialised rather than omitted so an unresolved item cannot be mistaken for a pre-#413 response, all three DRC categories walked, ERC shape byte-for-byte unchanged. The fixture README's provenance split (reporter's KiCad bytes / KiCad-resaved additions / two parser-only report entries) is exactly the honesty the fixture rule asks for.

What I ran here (Windows, KiCad 10.0.x):

worktree on 47672fa, Windows, kicad-cli 10.0.x
cargo test -p konnect-core --lib drc_ownership                  13 passed, 1 ignored
cargo test -p konnect-sexp --lib                                181 passed
cargo fmt / clippy -D warnings / --lib --tests / asset_references   all exit 0
KICAD_CLI=<install> cargo test ... run_drc_enriches_items_from_the_board_it_ran_on -- --ignored
                                                                ok (1 passed)

So the live half now has a second observation beyond your Windows run: the real kicad-cli pcb drc on the fixture board, through run_drc, answers ownership for every item and puts J1's cutouts and pads on J1. Fixture check: the .kicad_pcb is byte-for-byte pcbnew output (tab-indented, CRLF, generator "pcbnew"), as the README says.

Neuter, reproduced independently of tonydzi's: restoring first-wins in index_into (Entry::Occupied(_) => {}) makes a_duplicate_uuid_is_ambiguous_regardless_of_file_order fail; restoring the guard, green.

One change requested before merge — unrelated cleanup in cli.rs:

The diff deletes the doc comments on resolve_cli_executable and on four of the cli_discovery_tests (the three-case contract from #475: why empty stays empty for the sixty-odd "" fixtures, why an explicit missing path is not silently replaced by a discovered KiCad, and why the_bare_default_name_resolves_to_an_installed_kicad_cli skips rather than fails). Nothing in this PR touches that function's behaviour, and the replacement two-line comment loses the reasons. I suspect it is a reconstruction artefact from resolving #439's pre-#475 base against current main — please restore those comments verbatim from main so the PR's cli.rs hunk is only the ownership work. This is the "no unrelated cleanup" line of the checklist rather than a code defect.

Validation debt, your call (from tonydzi's second review): the read-failure arm at run_drc (tokio::fs::read_to_string → Err) is one of the two arms that call mark_ownership_unavailable, and only the parse-failure arm is proven by a test. Under the #446 risk-proportionate rule it is not a safety property — the worst case is a missing diagnostic, never a wrong owner — so I am fine landing it as named debt. If you would rather close it, the cheap seam is to have run_drc hand enrich_drc_items an io::Result<String> (or a small fn enrich_from_read(report, Result<String, io::Error>)) so the test can inject the read error without a kicad-cli. Either way, say which in the PR body.

After the comment restore and green checks on the new head, this is status:ready-to-merge from my side. Closes #413; no successor in this chain.

[neusse]

Queue refresh — this maintainer-owned reconstruction is second, immediately after #502. The feature behavior for #413 remains accepted; no redesign is required.

The remaining work on unchanged head 47672fa0634347e524b35f05bbe9940b8e837fa7 is narrow: restore the unrelated resolve_cli_executable and CLI-discovery-test comments verbatim from current main, and either add the small injectable read-failure test seam or explicitly record that branch as accepted validation debt in the PR body. Then reconstruct once onto the main produced by #502, preserve all migration/tool-directory entries, run all ten checks, and return the exact head SHA.

This branch is currently conflicting and 56 commits behind. status:waiting-on-author is correct; the next actor is us as the PR author.

[neusse]

Maintainer refresh complete on exact head 7397ec1ddd9901e46610e7c3ce2e1029374e8854.

• Reconstructed once onto current main after feat(library): draw a symbol body with graphics primitives #502 (52e8ef888d48d5a2f1449874e0d2b81ad041a925).
• Preserved Anton Dziatkovskii as the commit author.
• Restored the unrelated resolve_cli_executable and CLI-discovery comments verbatim from current main.
• Preserved all current API migration and tool-directory entries while adding the DRC ownership contract.
• Recorded the unreadable-board test arm as accepted validation debt in the PR body, under the risk-proportionate evidence rule.
• Local full gate passed: fmt, workspace clippy with warnings denied, workspace lib/tests, and workspace doc tests.

Hosted CI is running now. Once every required check is green, this head is ready for the final merge gate.

Stale exact-head review dismissed after its requested comment restoration was completed and its permitted validation-debt alternative was documented on refreshed head 7397ec1. All ten required checks are green.