master
John Lauer 1.0.61: Every Konnect v0.13.0 capability, 211 verbs (was 133; docs/konnect-gap.md maps all 228 Konnect tools). Schemati… baad156 14d ago
123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283284285286287288289290291292293294295296297298299300301302303304305306307308309310311312313314315316317318319320321322323324325326327328329330331332333334335336337338339340341342343344345346347348349350351352353354355356357358359360361362363364365366367368369370371372373374375376377378379380381382383384385386387388389390391392393394395396397398399400401402403404405406407408409410411412413414415416417418419420421422423424425426427428429430431432433434435436437438439440441442443444445446447448449450451452453454455456457458459460461462463464465466467468469470471472473474475476477478479480481482483484485486487488489490491492493494495496497498499500501502503504505506507508509510511512513514515516517518519520521522523524525526527528529530531532533534535536537538539540541542543544545546547548549550551552553554555556557558559560561562563564565566567568569570571572573574575576577578579580581582583584585586587588589590591592593594595596597598599600601602603604605606607608609610611612613614615616617618619620621622623624625626627628629630631632633634635636637638639640641642643644645646647648649650651652653654655656657658659660661662663664665666667668669670671672673674675676677678679680681682683684685686687688
//! Schematic design review from the netlist: decoupling on IC power pins, I2C pull-ups,
//! floating reset and enable pins, LED series resistors, regulator input and output caps,
//! bulk capacitance per rail, and annotation (unannotated or duplicate references).
//!
//! The checks read a `ReviewModel`: parts with their pins (number, name, electrical type,
//! net) and the nets. It is built either from kicad-cli's netlist (`from_netlist`, the
//! authoritative compile, when kicad-cli is installed) or from the files themselves
//! (`from_hierarchy`, this bridge's own connectivity across the sheet tree: global labels
//! and power symbols by name, hierarchical labels to their sheet pins).

use std::collections::{BTreeMap, BTreeSet, HashMap};

use serde::Serialize;
use serde_json::{json, Value};

use crate::netlist::Netlist;
use crate::sch_hier::Hierarchy;
use crate::sch_model::{self, ref_order, Connectivity, Dsu, Item};

#[derive(Clone, Debug, Serialize)]
pub struct RPin {
    pub number: String,
    pub name: String,
    pub electrical: String,
    pub net: String,
}

#[derive(Clone, Debug, Serialize)]
pub struct RPart {
    pub reference: String,
    pub value: String,
    #[serde(rename = "libId")]
    pub lib_id: String,
    pub footprint: String,
    pub description: String,
    pub pins: Vec<RPin>,
}

#[derive(Clone, Debug, Default)]
pub struct ReviewModel {
    pub parts: Vec<RPart>,
    /// Net name to (part index, pin index).
    pub nets: BTreeMap<String, Vec<(usize, usize)>>,
    /// Net names known to come from a power symbol or a global power flag.
    pub power_named: BTreeSet<String>,
    /// Every placed reference occurrence: (reference, unit, where) for annotation checks.
    pub occurrences: Vec<(String, u32, String)>,
    pub source: &'static str,
    pub sheets: usize,
    pub notes: Vec<String>,
}

impl ReviewModel {
    fn index_nets(&mut self) {
        self.nets.clear();
        for (pi, p) in self.parts.iter().enumerate() {
            for (k, pin) in p.pins.iter().enumerate() {
                self.nets.entry(pin.net.clone()).or_default().push((pi, k));
            }
        }
    }
}

/// kicad-cli's kicadxml netlist: authoritative (hierarchy, buses, every rule eeschema has).
pub fn from_netlist(n: &Netlist) -> ReviewModel {
    let mut parts: Vec<RPart> = n
        .components
        .iter()
        .map(|c| RPart { reference: c.reference.clone(), value: c.value.clone(), lib_id: c.libsource.clone(), footprint: c.footprint.clone(), description: c.description.clone(), pins: vec![] })
        .collect();
    let index: HashMap<String, usize> = parts.iter().enumerate().map(|(i, p)| (p.reference.clone(), i)).collect();
    for net in &n.nets {
        for node in &net.nodes {
            if let Some(&i) = index.get(&node.reference) {
                let name = node.pinfunction.strip_suffix(&format!("_{}", node.pin)).unwrap_or(&node.pinfunction).to_string();
                parts[i].pins.push(RPin { number: node.pin.clone(), name, electrical: node.pintype.clone(), net: net.name.clone() });
            }
        }
    }
    // the netlist folds parts sharing a reference into one: annotation reads the files
    // (`occurrences`) when the caller can load them
    let occurrences = n.components.iter().map(|c| (c.reference.clone(), 1, String::new())).collect();
    let notes = vec!["Compiled by kicad-cli. Duplicate references are folded into one part by the netlist, so the annotation check reads the sheet files.".to_string()];
    let mut m = ReviewModel { parts, occurrences, source: "netlist", sheets: 0, notes, ..Default::default() };
    m.index_nets();
    m
}

/// Every placed (non-power) symbol in the design, per sheet instance: (reference, unit, where).
pub fn occurrences(h: &Hierarchy) -> Vec<(String, u32, String)> {
    let mut out = Vec::new();
    for (i, inst) in h.instances.iter().enumerate() {
        let Some(s) = h.sch(i) else { continue };
        for sym in sch_model::extract(s, Some(&inst.path)).symbols.into_iter().filter(|x| !x.is_power) {
            out.push((sym.reference, sym.unit, format!("{} {}", inst.human, sym.uuid)));
        }
    }
    out
}

/// The bridge's own compile of the sheet tree.
pub fn from_hierarchy(h: &Hierarchy) -> ReviewModel {
    struct Sheet {
        model: sch_model::SheetModel,
        conn: Connectivity,
        offset: usize,
    }
    let mut sheets: Vec<Option<Sheet>> = Vec::new();
    let mut total = 0;
    for (i, inst) in h.instances.iter().enumerate() {
        match h.sch(i) {
            Some(s) => {
                let model = sch_model::extract(s, Some(&inst.path));
                let conn = Connectivity::build(&model);
                let n = conn.groups.len();
                sheets.push(Some(Sheet { model, conn, offset: total }));
                total += n;
            }
            None => sheets.push(None),
        }
    }
    let mut d = Dsu::new(total);
    // global and power names join across sheets
    let mut global: HashMap<String, usize> = HashMap::new();
    let mut power_named = BTreeSet::new();
    for s in sheets.iter().flatten() {
        for (g, grp) in s.conn.groups.iter().enumerate() {
            for (k, t) in &grp.names {
                if k == "global" || k == "power" {
                    if k == "power" {
                        power_named.insert(t.clone());
                    }
                    let id = s.offset + g;
                    match global.get(t) {
                        Some(&first) => d.union(first, id),
                        None => {
                            global.insert(t.clone(), id);
                        }
                    }
                }
            }
        }
    }
    // hierarchical labels in a child join the parent's sheet pin of the same name
    for (i, inst) in h.instances.iter().enumerate() {
        let (Some(child), Some(parent), Some(suuid)) = (sheets[i].as_ref(), inst.parent, inst.sheet_uuid.as_ref()) else { continue };
        let Some(par) = sheets[parent].as_ref() else { continue };
        let sps: Vec<&sch_model::SheetPin> = par.model.sheet_pins().collect();
        for (li, l) in child.model.labels.iter().enumerate() {
            if l.kind != "hierarchical" {
                continue;
            }
            let Some(spi) = sps.iter().position(|sp| sp.sheet_uuid == *suuid && sp.name == l.text) else { continue };
            let (Some(&cg), Some(&pg)) = (child.conn.of.get(&Item::Label(li)), par.conn.of.get(&Item::SheetPin(spi))) else { continue };
            d.union(child.offset + cg, par.offset + pg);
        }
    }
    // names per global net: power/global first, then the shallowest local or hierarchical label
    let mut best: HashMap<usize, (u8, usize, String)> = HashMap::new();
    for (i, s) in sheets.iter().enumerate() {
        let Some(s) = s else { continue };
        let depth = h.instances[i].depth;
        let human = h.instances[i].human.clone();
        for (g, grp) in s.conn.groups.iter().enumerate() {
            let root = d.find(s.offset + g);
            for (k, t) in &grp.names {
                let (rank, name) = match k.as_str() {
                    "power" | "global" => (0u8, t.clone()),
                    "local" => (1, format!("{human}{t}")),
                    "hierarchical" => (2, format!("{human}{t}")),
                    _ => continue,
                };
                let cand = (rank, depth, name);
                let e = best.entry(root).or_insert_with(|| cand.clone());
                if (cand.0, cand.1, &cand.2) < (e.0, e.1, &e.2) {
                    *e = cand;
                }
            }
        }
    }
    // parts and pins
    let mut parts: Vec<RPart> = Vec::new();
    let mut by_ref: HashMap<String, usize> = HashMap::new();
    let mut occurrences = Vec::new();
    let mut members: HashMap<usize, Vec<(String, String)>> = HashMap::new();
    for s in sheets.iter().flatten() {
        for p in s.model.pins.iter().filter(|p| !p.power_symbol) {
            let i = s.model.pins.iter().position(|q| std::ptr::eq(q, p)).unwrap_or(0);
            if let Some(&g) = s.conn.of.get(&Item::Pin(i)) {
                members.entry(d.find(s.offset + g)).or_default().push((p.reference.clone(), p.number.clone()));
            }
        }
    }
    for (i, s) in sheets.iter().enumerate() {
        let Some(s) = s else { continue };
        let human = h.instances[i].human.clone();
        for sym in s.model.symbols.iter().filter(|x| !x.is_power) {
            occurrences.push((sym.reference.clone(), sym.unit, format!("{human} {}", sym.uuid)));
            let pi = *by_ref.entry(sym.reference.clone()).or_insert_with(|| {
                parts.push(RPart { reference: sym.reference.clone(), value: sym.value.clone(), lib_id: sym.lib_id.clone(), footprint: sym.footprint.clone(), description: sym.description.clone(), pins: vec![] });
                parts.len() - 1
            });
            for (k, p) in s.model.pins.iter().enumerate().filter(|(_, p)| p.symbol_uuid == sym.uuid) {
                let Some(&g) = s.conn.of.get(&Item::Pin(k)) else { continue };
                let root = d.find(s.offset + g);
                let net = match best.get(&root) {
                    Some((_, _, n)) => n.clone(),
                    None => {
                        let mut ms = members.get(&root).cloned().unwrap_or_default();
                        ms.sort_by(|a, b| ref_order(&a.0).cmp(&ref_order(&b.0)).then(a.1.cmp(&b.1)));
                        ms.dedup();
                        if ms.len() <= 1 {
                            format!("unconnected-({}-Pad{})", p.reference, p.number)
                        } else {
                            format!("Net-({}-Pad{})", ms[0].0, ms[0].1)
                        }
                    }
                };
                if !parts[pi].pins.iter().any(|x| x.number == p.number) {
                    parts[pi].pins.push(RPin { number: p.number.clone(), name: p.name.clone(), electrical: p.electrical.clone(), net });
                }
            }
        }
    }
    let missing: Vec<String> = h.instances.iter().filter_map(|i| i.problem.as_ref().map(|p| format!("{}: {p}", i.human))).collect();
    let mut notes = vec!["Compiled by the bridge from the files (no kicad-cli): global labels and power symbols join by name, hierarchical labels join their sheet pins, hidden power pins join the net of their name. Buses are not expanded.".to_string()];
    if !missing.is_empty() {
        notes.push(format!("Sheets not reviewed: {}", missing.join("; ")));
    }
    let mut m = ReviewModel { parts, power_named, occurrences, source: "file", sheets: sheets.iter().flatten().count(), notes, ..Default::default() };
    m.index_nets();
    m
}

// ---------------------------------------------------------------------------
// Classification
// ---------------------------------------------------------------------------

fn tail_name(net: &str) -> String {
    net.rsplit('/').next().unwrap_or(net).to_ascii_uppercase()
}

pub fn is_ground(net: &str) -> bool {
    let n = tail_name(net);
    matches!(n.as_str(), "GND" | "GNDA" | "GNDD" | "GNDPWR" | "GNDREF" | "AGND" | "DGND" | "PGND" | "SGND" | "VSS" | "VSSA" | "0V" | "EARTH" | "CHASSIS")
        || n.starts_with("GND")
        || n.ends_with("GND")
}

fn looks_like_rail(net: &str) -> bool {
    let n = tail_name(net);
    if n.starts_with('+') || n.starts_with('-') && n.chars().nth(1).map_or(false, |c| c.is_ascii_digit()) {
        return true;
    }
    for p in ["VCC", "VDD", "VBUS", "VBAT", "VIN", "VSYS", "VREF", "VIO", "VAA", "AVDD", "DVDD", "IOVDD", "VMOT", "VOUT", "VREG"] {
        if n.starts_with(p) {
            return true;
        }
    }
    // 3V3, 5V, 12V, 1V8, V3V3
    let core = n.trim_start_matches('V');
    let mut chars = core.chars().peekable();
    let mut digits = 0;
    while chars.peek().map_or(false, |c| c.is_ascii_digit()) {
        chars.next();
        digits += 1;
    }
    digits > 0 && chars.next() == Some('V') && chars.all(|c| c.is_ascii_digit() || c == '_' || c.is_ascii_alphabetic())
}

#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum PartKind {
    Capacitor,
    Resistor,
    Led,
    Regulator,
    Ic,
    Connector,
    Other,
}

fn prefix(r: &str) -> String {
    r.chars().take_while(|c| c.is_ascii_alphabetic()).collect::<String>().to_ascii_uppercase()
}

pub fn kind(p: &RPart) -> PartKind {
    let pre = prefix(&p.reference);
    let lib = p.lib_id.to_ascii_lowercase();
    let sym = lib.rsplit(':').next().unwrap_or(&lib).to_string();
    let val = p.value.to_ascii_lowercase();
    if lib.contains("led") && (pre == "D" || pre == "LED") || sym.starts_with("led") {
        return PartKind::Led;
    }
    if pre == "C" || sym == "c" || sym.starts_with("c_") || sym == "cp" || sym.starts_with("cp_") {
        return PartKind::Capacitor;
    }
    if pre == "R" || sym == "r" || sym.starts_with("r_") {
        return PartKind::Resistor;
    }
    if pre == "J" || pre == "P" || pre == "CN" || lib.starts_with("connector") {
        return PartKind::Connector;
    }
    if lib.starts_with("regulator_") || val.contains("ldo") || val.contains("1117") || lib.contains("regulator") {
        return PartKind::Regulator;
    }
    if pre == "U" || pre == "IC" {
        let has_in = p.pins.iter().any(|x| matches!(norm_pin(&x.name).as_str(), "VIN" | "VI" | "IN"));
        let has_out = p.pins.iter().any(|x| matches!(norm_pin(&x.name).as_str(), "VOUT" | "VO" | "OUT") && x.electrical == "power_out");
        return if has_in && has_out { PartKind::Regulator } else { PartKind::Ic };
    }
    PartKind::Other
}

/// Pin name without KiCad's overbar markup and separators: `~{RESET}` -> RESET.
pub fn norm_pin(name: &str) -> String {
    name.replace("~{", "").replace(['}', '~', '/', '#', ' '], "").to_ascii_uppercase()
}

/// Farads from a value like 100n, 4.7uF, 10µ, 1u0, 22pF; None when unreadable.
pub fn farads(v: &str) -> Option<f64> {
    let s = v.trim().replace('µ', "u").replace('μ', "u").to_ascii_lowercase();
    let s = s.split([' ', '/', ',']).next().unwrap_or("");
    let s = s.trim_end_matches('f');
    let mut num = String::new();
    let mut mult = 1.0;
    let mut seen_unit = false;
    for c in s.chars() {
        if c.is_ascii_digit() || c == '.' {
            num.push(c);
        } else if !seen_unit {
            mult = match c {
                'p' => 1e-12,
                'n' => 1e-9,
                'u' => 1e-6,
                'm' => 1e-3,
                _ => return None,
            };
            if !num.contains('.') {
                num.push('.');
            }
            seen_unit = true;
        } else {
            return None;
        }
    }
    if !seen_unit {
        // a bare number on a capacitor is farads only in theory; KiCad users mean the unit they wrote
        return None;
    }
    let x: f64 = num.trim_end_matches('.').parse().ok()?;
    Some(x * mult)
}

fn fmt_f(f: f64) -> String {
    if f >= 1e-6 {
        format!("{}u", crate::schematic::fmt_num(f / 1e-6))
    } else if f >= 1e-9 {
        format!("{}n", crate::schematic::fmt_num(f / 1e-9))
    } else {
        format!("{}p", crate::schematic::fmt_num(f / 1e-12))
    }
}

#[derive(Clone, Debug, Serialize)]
pub struct ReviewFinding {
    pub check: &'static str,
    /// error | warning | info
    pub severity: &'static str,
    pub refs: Vec<String>,
    pub nets: Vec<String>,
    pub message: String,
    pub why: String,
    pub fix: String,
}

pub const CHECKS: &[&str] = &["decoupling", "i2c_pullups", "reset_enable", "led_resistor", "regulator_caps", "bulk_capacitance", "annotation"];

struct Ctx<'a> {
    m: &'a ReviewModel,
    rails: BTreeSet<String>,
}

impl<'a> Ctx<'a> {
    fn new(m: &'a ReviewModel) -> Ctx<'a> {
        let mut rails = BTreeSet::new();
        for (net, members) in &m.nets {
            if is_ground(net) {
                continue;
            }
            let has_power_out = members.iter().any(|(p, k)| m.parts[*p].pins[*k].electrical == "power_out");
            if m.power_named.contains(net) || looks_like_rail(net) || has_power_out {
                rails.insert(net.clone());
            }
        }
        Ctx { m, rails }
    }
    fn parts_on(&self, net: &str) -> Vec<(usize, usize)> {
        self.m.nets.get(net).cloned().unwrap_or_default()
    }
    /// Two-terminal parts of `kind` with one pin on `net`: (part, the other pin's net).
    fn two_terminal(&self, net: &str, want: PartKind) -> Vec<(usize, String)> {
        let mut out = Vec::new();
        for (pi, k) in self.parts_on(net) {
            let p = &self.m.parts[pi];
            if kind(p) != want || p.pins.len() != 2 {
                continue;
            }
            let other = &p.pins[1 - k].net;
            if other != net {
                out.push((pi, other.clone()));
            }
        }
        out
    }
    fn caps_to_ground(&self, net: &str) -> Vec<(usize, Option<f64>)> {
        self.two_terminal(net, PartKind::Capacitor).into_iter().filter(|(_, o)| is_ground(o)).map(|(p, _)| (p, farads(&self.m.parts[p].value))).collect()
    }
    fn single(&self, net: &str) -> bool {
        net.starts_with("unconnected-") || self.parts_on(net).len() <= 1
    }
}

pub fn review(m: &ReviewModel, only: Option<&[String]>) -> Vec<ReviewFinding> {
    let on = |c: &str| only.map_or(true, |o| o.iter().any(|x| x == c));
    let cx = Ctx::new(m);
    let mut out = Vec::new();

    if on("decoupling") {
        for p in m.parts.iter().filter(|p| kind(p) == PartKind::Ic) {
            let mut nets: BTreeMap<&str, Vec<String>> = BTreeMap::new();
            for pin in p.pins.iter().filter(|x| x.electrical == "power_in" && !is_ground(&x.net) && !x.net.starts_with("unconnected-")) {
                nets.entry(pin.net.as_str()).or_default().push(format!("{} {}", pin.number, pin.name));
            }
            for (net, pins) in nets {
                if cx.caps_to_ground(net).is_empty() {
                    out.push(ReviewFinding {
                        check: "decoupling",
                        severity: "warning",
                        refs: vec![p.reference.clone()],
                        nets: vec![net.to_string()],
                        message: format!("{} power pin(s) {} on {net} have no decoupling capacitor to ground", p.reference, pins.join(", ")),
                        why: "An IC draws its switching current in fast edges; without a local capacitor that current comes through the rail's inductance and the supply at the pin droops and rings.".into(),
                        fix: format!("Place a 100 nF ceramic from {net} to GND for each power pin group of {}, beside the pin on the board (kicad_sch_place_symbol Device:C, then kicad_sch_wire both ends).", p.reference),
                    });
                }
            }
        }
    }

    if on("i2c_pullups") {
        for net in m.nets.keys() {
            let tail = tail_name(net);
            let toks: Vec<&str> = tail.split(|c: char| !c.is_ascii_alphanumeric()).collect();
            let named = toks.iter().any(|t| t.starts_with("SDA") || t.starts_with("SCL"));
            let by_pin = cx.parts_on(net).iter().any(|(p, k)| matches!(norm_pin(&m.parts[*p].pins[*k].name).as_str(), "SDA" | "SCL" | "I2C_SDA" | "I2C_SCL"));
            if !(named || by_pin) || cx.single(net) {
                continue;
            }
            let pullups: Vec<usize> = cx.two_terminal(net, PartKind::Resistor).into_iter().filter(|(_, o)| cx.rails.contains(o)).map(|(p, _)| p).collect();
            if pullups.is_empty() {
                let line = if tail.contains("SCL") { "SCL" } else { "SDA" };
                out.push(ReviewFinding {
                    check: "i2c_pullups",
                    severity: "warning",
                    refs: cx.parts_on(net).iter().map(|(p, _)| m.parts[*p].reference.clone()).collect::<BTreeSet<_>>().into_iter().collect(),
                    nets: vec![net.clone()],
                    message: format!("I2C line {net} has no pull-up resistor to a supply rail"),
                    why: format!("I2C drivers are open drain: they only pull low. Without a pull-up, {line} never returns high and the bus does not work (unless the pull-up sits on another board, which the schematic should say)."),
                    fix: format!("Add a resistor from {net} to the I/O rail (4.7k for 100 kHz, 2.2k for 400 kHz): kicad_sch_place_symbol Device:R and wire it, once per bus."),
                });
            }
        }
    }

    if on("reset_enable") {
        const NAMES: &[&str] = &["RESET", "RST", "NRST", "RESETN", "RSTN", "NRESET", "MCLR", "EN", "ENABLE", "CE", "CHIP_EN", "CHIPEN", "SHDN", "NSHDN", "SHDNN", "PWRDN", "ON", "ONOFF", "RUN"];
        for p in m.parts.iter().filter(|p| matches!(kind(p), PartKind::Ic | PartKind::Regulator)) {
            for pin in &p.pins {
                let n = norm_pin(&pin.name);
                let hit = NAMES.contains(&n.as_str()) || n.starts_with("RESET") || n.starts_with("NRST") || n.starts_with("RST");
                if !hit || matches!(pin.electrical.as_str(), "output" | "power_out" | "no_connect") {
                    continue;
                }
                if cx.single(&pin.net) {
                    out.push(ReviewFinding {
                        check: "reset_enable",
                        severity: "error",
                        refs: vec![p.reference.clone()],
                        nets: vec![pin.net.clone()],
                        message: format!("{}.{} {} is floating", p.reference, pin.number, pin.name),
                        why: format!("A floating {} input picks up noise: the part may never start, or reset and shut down at random.", pin.name),
                        fix: format!("Tie {}.{} to its active state (to the supply for an enable that should always be on, or through a 10k pull-up with a 100 nF to ground for a reset), or drive it from a GPIO.", p.reference, pin.number),
                    });
                    continue;
                }
                let defined = cx.rails.contains(&pin.net)
                    || is_ground(&pin.net)
                    || cx.parts_on(&pin.net).iter().any(|(q, k)| {
                        let qp = &m.parts[*q];
                        let e = qp.pins[*k].electrical.as_str();
                        (qp.reference != p.reference && matches!(e, "output" | "bidirectional" | "open_collector" | "open_emitter" | "power_out" | "tri_state"))
                            || matches!(kind(qp), PartKind::Resistor | PartKind::Connector)
                    });
                if !defined {
                    out.push(ReviewFinding {
                        check: "reset_enable",
                        severity: "warning",
                        refs: vec![p.reference.clone()],
                        nets: vec![pin.net.clone()],
                        message: format!("Nothing sets the level of {}.{} {} (net {})", p.reference, pin.number, pin.name, pin.net),
                        why: "The net reaches the pin but no driver, pull resistor, rail or connector defines it, so it floats like an open pin.".into(),
                        fix: format!("Add a pull resistor on {} to the inactive level, or drive it from a GPIO or a supervisor.", pin.net),
                    });
                }
            }
        }
    }

    if on("led_resistor") {
        for (pi, p) in m.parts.iter().enumerate().filter(|(_, p)| kind(p) == PartKind::Led) {
            let limited = p.pins.iter().any(|pin| cx.parts_on(&pin.net).iter().any(|(q, _)| *q != pi && kind(&m.parts[*q]) == PartKind::Resistor));
            if !limited {
                out.push(ReviewFinding {
                    check: "led_resistor",
                    severity: "warning",
                    refs: vec![p.reference.clone()],
                    nets: p.pins.iter().map(|x| x.net.clone()).collect(),
                    message: format!("LED {} has no series resistor on either side", p.reference),
                    why: "An LED's current rises steeply above its forward voltage: across a rail it draws whatever the supply gives and burns out (or overloads the driving pin).".into(),
                    fix: format!("Put a resistor in series with {}: R = (Vsupply - Vf) / I, for example 330R to 1k at 3.3 V; skip only if a constant-current driver feeds it.", p.reference),
                });
            }
        }
    }

    if on("regulator_caps") {
        for p in m.parts.iter().filter(|p| kind(p) == PartKind::Regulator) {
            let inputs: BTreeSet<&str> = p.pins.iter().filter(|x| (x.electrical == "power_in" || matches!(norm_pin(&x.name).as_str(), "VIN" | "VI" | "IN")) && !is_ground(&x.net) && !x.net.starts_with("unconnected-")).map(|x| x.net.as_str()).collect();
            let outputs: BTreeSet<&str> = p.pins.iter().filter(|x| x.electrical == "power_out" || matches!(norm_pin(&x.name).as_str(), "VOUT" | "VO" | "OUT")).filter(|x| !x.net.starts_with("unconnected-")).map(|x| x.net.as_str()).collect();
            let inputs: Vec<&str> = inputs.difference(&outputs).copied().collect();
            for net in inputs {
                if cx.caps_to_ground(net).is_empty() {
                    out.push(ReviewFinding {
                        check: "regulator_caps",
                        severity: "warning",
                        refs: vec![p.reference.clone()],
                        nets: vec![net.to_string()],
                        message: format!("Regulator {} has no input capacitor on {net}", p.reference),
                        why: "A regulator's input needs a low-impedance local source; with long supply wiring the input rings and the regulator can oscillate or drop out on load steps.".into(),
                        fix: format!("Add the input capacitor the datasheet asks for (typically 1 uF to 10 uF ceramic) from {net} to GND beside {}.", p.reference),
                    });
                }
            }
            for net in outputs {
                let caps = cx.caps_to_ground(net);
                let biggest = caps.iter().filter_map(|(_, f)| *f).fold(None, |a: Option<f64>, f| Some(a.map_or(f, |x| x.max(f))));
                let unknown = caps.iter().any(|(_, f)| f.is_none());
                if caps.is_empty() {
                    out.push(ReviewFinding {
                        check: "regulator_caps",
                        severity: "warning",
                        refs: vec![p.reference.clone()],
                        nets: vec![net.to_string()],
                        message: format!("Regulator {} has no output capacitor on {net}", p.reference),
                        why: "Most LDOs are only stable with a minimum output capacitance (often 1 uF or more, sometimes with an ESR window); without it the output can oscillate.".into(),
                        fix: format!("Add the datasheet's output capacitor (typically 1 uF to 10 uF ceramic) from {net} to GND beside {}.", p.reference),
                    });
                } else if !unknown && biggest.map_or(false, |b| b < 0.99e-6) {
                    out.push(ReviewFinding {
                        check: "regulator_caps",
                        severity: "warning",
                        refs: vec![p.reference.clone()],
                        nets: vec![net.to_string()],
                        message: format!("Regulator {} output {net} has only {} of capacitance", p.reference, fmt_f(biggest.unwrap_or(0.0))),
                        why: "Small decoupling capacitors elsewhere on the rail do not meet an LDO's stability minimum, which is usually 1 uF or more at the output pin.".into(),
                        fix: format!("Add a 1 uF to 10 uF ceramic from {net} to GND beside {} (check the datasheet's minimum and ESR range).", p.reference),
                    });
                }
            }
        }
    }

    if on("bulk_capacitance") {
        for net in &cx.rails {
            let loads = cx.parts_on(net).iter().filter(|(p, k)| m.parts[*p].pins[*k].electrical == "power_in" && matches!(kind(&m.parts[*p]), PartKind::Ic | PartKind::Regulator)).count();
            if loads == 0 {
                continue;
            }
            let caps = cx.caps_to_ground(net);
            if caps.iter().any(|(_, f)| f.map_or(true, |f| f >= 4.7e-6 * 0.99)) {
                continue;
            }
            let biggest = caps.iter().filter_map(|(_, f)| *f).fold(0.0, f64::max);
            out.push(ReviewFinding {
                check: "bulk_capacitance",
                severity: "info",
                refs: caps.iter().map(|(p, _)| m.parts[*p].reference.clone()).collect(),
                nets: vec![net.clone()],
                message: if caps.is_empty() { format!("Rail {net} has no capacitance to ground at all") } else { format!("Rail {net} has no bulk capacitor (largest is {})", fmt_f(biggest)) },
                why: "Decoupling capacitors handle fast edges; a rail also needs bulk capacitance (about 4.7 uF to 100 uF where it enters or is generated) for load steps and to keep the source impedance low.".into(),
                fix: format!("Add a 10 uF (or larger) capacitor from {net} to GND where the rail enters the board or leaves its regulator."),
            });
        }
    }

    if on("annotation") {
        let mut seen: BTreeMap<(String, u32), Vec<String>> = BTreeMap::new();
        let mut unannotated = BTreeSet::new();
        for (r, unit, wh) in &m.occurrences {
            if r.contains('?') || r.is_empty() {
                unannotated.insert(r.clone());
                continue;
            }
            seen.entry((r.clone(), *unit)).or_default().push(wh.clone());
        }
        for r in unannotated {
            out.push(ReviewFinding {
                check: "annotation",
                severity: "error",
                refs: vec![r.clone()],
                nets: vec![],
                message: format!("{} is not annotated", if r.is_empty() { "A symbol" } else { r.as_str() }),
                why: "An unannotated reference cannot become a footprint on the board; the netlist and BOM drop or merge it.".into(),
                fix: "Annotate (in the Schematic Editor: Tools > Annotate Schematic), or give it a reference with kicad_sch_set_property {name:\"Reference\"}.".into(),
            });
        }
        let mut dups: Vec<(String, usize)> = seen.iter().filter(|(_, v)| v.len() > 1).map(|((r, _), v)| (r.clone(), v.len())).collect();
        dups.sort();
        dups.dedup();
        for (r, n) in dups {
            out.push(ReviewFinding {
                check: "annotation",
                severity: "error",
                refs: vec![r.clone()],
                nets: vec![],
                message: format!("Reference {r} is used by {n} different symbols"),
                why: "Two parts with one reference become one footprint on the board: one of them silently disappears and their nets merge in the netlist.".into(),
                fix: format!("Renumber one of them (kicad_sch_set_property {{reference:\"{r}\", name:\"Reference\", value:\"<free ref>\"}} on its sheet), or re-annotate."),
            });
        }
    }
    out
}

pub fn summary(m: &ReviewModel, f: &[ReviewFinding]) -> Value {
    let count = |s: &str| f.iter().filter(|x| x.severity == s).count();
    let mut by_check = serde_json::Map::new();
    for c in CHECKS {
        by_check.insert(c.to_string(), json!(f.iter().filter(|x| x.check == *c).count()));
    }
    json!({
        "errors": count("error"),
        "warnings": count("warning"),
        "info": count("info"),
        "byCheck": by_check,
        "parts": m.parts.len(),
        "nets": m.nets.len(),
    })
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn farads_parse() {
        assert!((farads("100n").unwrap() - 100e-9).abs() < 1e-15);
        assert!((farads("4.7uF").unwrap() - 4.7e-6).abs() < 1e-12);
        assert!((farads("4u7").unwrap() - 4.7e-6).abs() < 1e-12);
        assert!((farads("10µF 16V").unwrap() - 10e-6).abs() < 1e-12);
        assert!((farads("22p").unwrap() - 22e-12).abs() < 1e-18);
        assert_eq!(farads("C"), None);
        assert_eq!(farads("10"), None);
    }

    #[test]
    fn ground_and_rails() {
        assert!(is_ground("GND") && is_ground("/Power/AGND") && is_ground("VSS") && !is_ground("+3V3"));
        assert!(looks_like_rail("+3V3") && looks_like_rail("VCC") && looks_like_rail("3V3") && looks_like_rail("/x/VDD_IO") && looks_like_rail("12V"));
        assert!(!looks_like_rail("SDA") && !looks_like_rail("Net-(R1-Pad1)"));
    }

    #[test]
    fn pin_names_normalise() {
        assert_eq!(norm_pin("~{RESET}"), "RESET");
        assert_eq!(norm_pin("EN"), "EN");
    }
}