fix: correct evidence_source on findings that claim datasheet provenance they don't have - #37
Open
fl4p wants to merge 2 commits into
Open
fix: correct evidence_source on findings that claim datasheet provenance they don't have#37fl4p wants to merge 2 commits into
fl4p wants to merge 2 commits into
Conversation
Five findings across two analyzers reported an evidence_source that
contradicts where the value actually came from. Provenance is what tells a
reviewer how much a finding is worth, so a wrong tag is worse than a missing
one -- it launders an API guess or a package average into datasheet evidence.
lifecycle_audit LC-005 (single source) and LC-006 (long lead time) hardcoded
evidence_source="datasheet". Both are derived purely from distributor API
responses; LC-006 reads src_data["lead_time"] and records lead_source="mouser"
two lines above the claim that a datasheet was the evidence. No datasheet
states a lead time. LC-001..LC-004 in the same file already use "api_lookup".
analyze_thermal TH-DET and TS-001/002/003/005 set evidence_source="datasheet"
when rtheta_ja_source == "package_table". That table is
PACKAGE_THERMAL_RESISTANCE -- generic per-package Rtheta_JA averages selected
by a regex over the footprint string. analyze_thermal:394 shows the source is
only ever "package_table" or "default", so there was no branch in which
"datasheet" was correct and the conditional itself was the bug.
lifecycle_audit LT-001 had the mirror defect: it mapped any non-empty
temperature source to "api_lookup", including "extraction_cache", which is a
real per-MPN datasheet extraction. The same analyzer was over-claiming
datasheet provenance in two rules and under-claiming it in a third. Added
_temp_evidence_source(): extraction_cache -> datasheet, api:* -> api_lookup,
everything else (e.g. the "mouser_suggestion" tag at :534) -> heuristic_rule.
Measured on a real 68-component board (24-bit ADC front end, 14 unique MPNs,
DigiKey + Mouser + element14 queried):
before by_evidence_source: {datasheet: 7, api_lookup: 6}
after by_evidence_source: {api_lookup: 13}
trust_level (low), lifecycle_summary (8 active / 6 unknown) and
temperature_summary (8 checked, 0 failing) are all unchanged -- only the
provenance labels move. api_lookup and heuristic_rule are both valid in
finding_schema.py and no key or type changes, so there is no schema drift.
The FastAdd example URL ended in &newcart=true. Per DigiKey's own FastAdd guide the default appends to the current cart while newcart=true starts a new empty one, and the shopping-cart page warns that creating a new cart deletes the items currently in it. An agent following this reference literally destroys whatever the user had already assembled -- and a cart is not recoverable from the API, since DigiKey exposes no cart read/write endpoint. Dropped the parameter from the example, documented it as deliberate opt-in in DigiKey's own wording, and linked the guide. Also stated the "/" separator convention for multiple reference designators. The existing paste example already used C1/C2/C5 but never said why, so it read as cosmetic. _write_digikey_order() in bom_manager.py already rewrites commas to "/", so generated files were never affected; this only matters for hand-assembled paste blocks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five findings across two analyzers reported an
evidence_sourcethat contradicts where the value actually came from. Provenance is what tells a reviewer how much a finding is worth, so a wrong tag is worse than a missing one — it launders an API guess or a package average into datasheet evidence.Over-claiming (3 sites)
lifecycle_auditLC-005 (single source) and LC-006 (long lead time) hardcodedevidence_source="datasheet". Both derive purely from distributor API responses — LC-006 readssrc_data["lead_time"]and recordslead_source="mouser"two lines above the claim that a datasheet was the evidence. No datasheet states a lead time. LC-001..LC-004 in the same file already use"api_lookup".analyze_thermalTH-DET and TS-001/002/003/005 setevidence_source="datasheet"whenrtheta_ja_source == "package_table". That table isPACKAGE_THERMAL_RESISTANCE— generic per-package Rθ_JA averages selected by a regex over the footprint string.analyze_thermal:394shows the source is only ever"package_table"or"default", so there was no branch in which"datasheet"was correct and the conditional itself was the bug.Under-claiming (1 site, the mirror defect)
lifecycle_auditLT-001 mapped any non-empty temperature source to"api_lookup", including"extraction_cache"— which is a real per-MPN datasheet extraction. The same analyzer was over-claiming datasheet provenance in two rules and under-claiming it in a third. Added_temp_evidence_source():extraction_cache → datasheet,api:* → api_lookup, everything else (e.g. the"mouser_suggestion"tag at:534) →heuristic_rule.Testing
Validated against
kicad-happy-testharnesson a 40-repo corpus subset (narrower than the recommendedquick_200= 261 repos), producing 382 thermal outputs.A/B,
mainvs this branch, same corpus and same schematic/PCB inputs. Every differing leaf field across all 382 outputs:Nothing else. No temperature, severity, score or count changed, and zero list-length differences — no detection added or removed. Aggregate shift:
{datasheet: 39, geometry: 73, heuristic_rule: 6}→{geometry: 73, heuristic_rule: 45}.Real-world impact: 39 findings and 48 assessments across 40 repos were claiming datasheet provenance for a footprint-regex package average.
Regression assertions (
regression/run_checks.py --type thermal): 9,932 total / 169 passed / 68 failed — identical onmainand on this branch, same failure set. Those 68 are pre-existing drift between the seeded assertions and currentmain, not introduced here.Re-seeding: none needed. No assertion targets
evidence_sourceas a finding field (0 hits for"field": "evidence_source"across 154,664 assertion files — theevidence_sourcekey in those files is the assertion's ownauto_seededprovenance, not the finding's), andbaselines/thermal.jsonrecords only summary counts with no findings array, verified across all 16,062 thermal baselines. No schema drift:api_lookupandheuristic_ruleare both valid infinding_schema.py, and no keys or types change.Not covered by the harness:
lifecycle_audithas no corpus runner (run/has schematic, pcb, gerbers, thermal, emc, spice, datasheets) — it needs network access and API keys. Its three changes were validated separately on a real 68-component board (24-bit ADC front end, 14 unique MPNs, DigiKey + Mouser + element14 queried):by_evidence_source {datasheet: 7, api_lookup: 6}→{api_lookup: 13}, withtrust_level,lifecycle_summaryandtemperature_summaryall unchanged. The newextraction_cache → datasheetbranch is unit-tested over all six source tags but not exercised end-to-end, as that board has nodatasheets/extracted/data.Two related defects found but deliberately left out
These change finding semantics rather than provenance, so they seem like your call rather than mine:
lifecycle_audit:731(LC-ACT) omitsconfidenceandevidence_sourceentirely — this is what forcestrust_level="low"viaunknown_confidence— and writes summary "active" for LCSC-onlyunknownstatus.lifecycle_audit:744(LC-005) —total_queriedcounts only APIs that returned rather than all attempted,status is Nonecounts as active, and LCSC returns no lifecycle status at all, so "only available from X of Y sources checked" is not supported by its own data.Happy to open either as a follow-up.
Second commit (unrelated, docs only)
skills/bom/references/ordering-and-fabrication.md— the FastAdd example URL ended in&newcart=true. Per DigiKey's own FastAdd guide the default appends to the current cart whilenewcart=truestarts a new empty one, and the cart page warns that creating a new cart deletes the items currently in it. An agent following this reference literally destroys whatever the user had already assembled — and it is not recoverable, since DigiKey exposes no cart read/write API endpoint. Dropped the parameter from the example and documented it as deliberate opt-in.Also stated the
/separator convention for multiple reference designators. The existing paste example already usedC1/C2/C5but never said why, so it read as cosmetic._write_digikey_order()inbom_manager.pyalready rewrites commas to/, so generated files were never affected — this only matters for hand-assembled paste blocks.