← Commit history

Merge PR #276: fab_bundle 1.1.0: correct drill and keep-out roles, strip the job file owner

AdityaAngajala ·143939e192 ·3h ago ·parent d398049
5 files changed +223−77
docs/fabrication.md+15−4
@@ -30,7 +30,10 @@ the current converter still consumes them. - `stackup`: known `thicknessMm`, `copperLayers`, `maskColor` and   `silkColor` by `top`/`bottom`, and `finish`. Unknown values stay null or   absent; never turn a viewer's default into claimed design data.-- `gerbers.included` is true. `drill.plating` says `split` or `mixed`.+- `gerbers.included` is true. `drill.plating` says `split`, `mixed`,+  `plated` (no unplated drill file) or `unplated`. A drill file's role comes+  from its X2 `FileFunction` when it has one, else from whole words of its+  name (`PTH`, `NPTH`), else it is mixed.   Exporters may also preserve `origin`, `outline`, `exportedAt` and their   own provenance. A historical backfill carries `backfill` and its original   `tier`; it must never claim to be a fresh native EDA export.@@ -52,9 +55,17 @@ Use the versions actually installed, not the example values. `--fab` also accepts an EDA-produced zip. `--stackup` is optional; pass the data the EDA provides. Repeat `--source` for every exported source, including every schematic sheet. The helper writes the canonical zip name without changing-its members' bytes. It refuses missing layers, a blank outline, missing drill-data and changed hashes. It does not prove alignment or validate the circuit;-the existing conversion and manufacturing checks still apply.+its members' bytes, with one exception: a Gerber job file loses its `Owner`+field, where Fusion writes the EDA account's name and email, because the+zip is published with the page. `check` refuses a job file that still has+one. The helper refuses missing layers, a blank outline, missing drill data,+changed hashes, two files for one required layer, two files of the same name+in different folders, layer files with names the board reader skips (use+KiCad or Protel extensions) and archives over 2000 files or 512 MiB+unpacked. A `.GKO` counts as the outline only when no other outline file+exists, since Altium writes its keep-out layer there. It does not prove+alignment or validate the circuit; the existing conversion and+manufacturing checks still apply.  For a bridge-produced record, verify it before any packaging rename. If the bridge used a design stem instead of the wiki slug, use a fresh staging
package.json+62−62
@@ -1,62 +1,62 @@-{
-  "slug": "molecule-publish",
-  "version": "2.11.0",
-  "type": "skill",
-  "description": "Publish molecules AND generic boards to the Adom wiki - one publisher, two shapes: OCCT conversion (molecule mode with stats gates, or plain for boards), component packaging, adom-wiki publish, APM render assets + component dependencies. Companion to adom-molecule-import.",
-  "tags": [
-    "molecule",
-    "pipeline",
-    "wiki",
-    "hydrogen",
-    "skill"
-  ],
-  "dependencies": {
-    "adom/adom-molecule": "^1.1.3"
-  },
-  "scripts": {
-    "install": "./install.sh",
-    "uninstall": "./uninstall.sh"
-  },
-  "hero": {
-    "headline": "Molecule bundle to wiki page",
-    "subhead": "OCCT convert, stats gates, package, publish - EDA-agnostic",
-    "screenshot": "docs/shot.png"
-  },
-  "discovery_triggers": [
-    "publish this molecule to the wiki",
-    "upload molecule bundle",
-    "convert and publish my molecule",
-    "molecule to the wiki",
-    "publish molecule package",
-    "publish my board",
-    "publish board to the wiki",
-    "board to project manager",
-    "render board for apm"
-  ],
-  "discovery_pitch": "Publish a molecule source bundle (STEP + schematic + board + metadata) to the Adom wiki: OCCT molecule-mode conversion with stats gates, component packaging, adom-wiki publish. EDA-agnostic middle of the pipeline; companion to adom-molecule-import.",
-  "sample_prompts": [
-    {
-      "label": "Publish a bundle",
-      "prompt": "I have ABC.step, ABC.kicad_pcb and ABC.kicad_sch - publish this molecule to the wiki"
-    },
-    {
-      "label": "Convert and publish",
-      "prompt": "Convert my molecule through OCCT and publish it"
-    }
-  ],
-  "files": [
-    "README.md",
-    "SKILL.md",
-    "docs/fabrication.md",
-    "install.sh",
-    "uninstall.sh",
-    "package.json",
-    "page.json",
-    "scripts/bundle_3d.py",
-    "scripts/component_graph.py",
-    "scripts/fab_bundle.py",
-    "scripts/hero_camera.py",
-    "scripts/render_parity.py",
-    "scripts/tested_with.py"
-  ]
-}
+{+  "slug": "molecule-publish",+  "version": "2.12.0",+  "type": "skill",+  "description": "Publish molecules AND generic boards to the Adom wiki - one publisher, two shapes: OCCT conversion (molecule mode with stats gates, or plain for boards), component packaging, adom-wiki publish, APM render assets + component dependencies. Companion to adom-molecule-import.",+  "tags": [+    "molecule",+    "pipeline",+    "wiki",+    "hydrogen",+    "skill"+  ],+  "dependencies": {+    "adom/adom-molecule": "^1.1.3"+  },+  "scripts": {+    "install": "./install.sh",+    "uninstall": "./uninstall.sh"+  },+  "hero": {+    "headline": "Molecule bundle to wiki page",+    "subhead": "OCCT convert, stats gates, package, publish - EDA-agnostic",+    "screenshot": "docs/shot.png"+  },+  "discovery_triggers": [+    "publish this molecule to the wiki",+    "upload molecule bundle",+    "convert and publish my molecule",+    "molecule to the wiki",+    "publish molecule package",+    "publish my board",+    "publish board to the wiki",+    "board to project manager",+    "render board for apm"+  ],+  "discovery_pitch": "Publish a molecule source bundle (STEP + schematic + board + metadata) to the Adom wiki: OCCT molecule-mode conversion with stats gates, component packaging, adom-wiki publish. EDA-agnostic middle of the pipeline; companion to adom-molecule-import.",+  "sample_prompts": [+    {+      "label": "Publish a bundle",+      "prompt": "I have ABC.step, ABC.kicad_pcb and ABC.kicad_sch - publish this molecule to the wiki"+    },+    {+      "label": "Convert and publish",+      "prompt": "Convert my molecule through OCCT and publish it"+    }+  ],+  "files": [+    "README.md",+    "SKILL.md",+    "docs/fabrication.md",+    "install.sh",+    "uninstall.sh",+    "package.json",+    "page.json",+    "scripts/bundle_3d.py",+    "scripts/component_graph.py",+    "scripts/fab_bundle.py",+    "scripts/hero_camera.py",+    "scripts/render_parity.py",+    "scripts/tested_with.py"+  ]+}
page.json+2−2
@@ -1,6 +1,6 @@ {   "slug": "molecule-publish",-  "version": "2.11.0",+  "version": "2.12.0",   "type": "skill",   "description": "Publish molecules AND generic boards to the Adom wiki - one publisher, two shapes: OCCT conversion (molecule mode with stats gates, or plain for boards), component packaging, adom-wiki publish, APM render assets + component dependencies. Companion to adom-molecule-import.",   "tags": [@@ -66,4 +66,4 @@     "scripts/render_parity.py",     "scripts/tested_with.py"   ]-}\ No newline at end of file+}
scripts/fab_bundle.py+86−9
@@ -8,7 +8,7 @@ import sys import zipfile from pathlib import Path, PurePosixPath -VERSION = "1.0.0"+VERSION = "1.1.0" REQUIRED = {"copper_top", "copper_bottom", "mask_top", "mask_bottom",             "silk_top", "silk_bottom", "outline"} DRILLS = {"drill_plated", "drill_unplated", "drill_mixed"}@@ -16,6 +16,11 @@ EXT_ROLES = {"gtl": "copper_top", "gbl": "copper_bottom", "gts": "mask_top",              "gbs": "mask_bottom", "gto": "silk_top", "gbo": "silk_bottom",              "gtp": "paste_top", "gbp": "paste_bottom", "gko": "outline",              "gm1": "outline", "gbrjob": "gerber_job"}+# The names board-look's fab.py reads from a fabrication zip (it keeps only these, by file name).+# Layer files must match it, and no two may share a file name, or the reader renders another file.+READER_EXT = re.compile(r"\.(gbr|gtl|gbl|gts|gbs|gto|gbo|gko|gm1|gml|gm|cmp|sol|stc|sts|plc|pls|drl|xln|txt|exc|gbrjob|g\d+|gp\d|gtp|gbp|dri|gpi)$", re.I)+ZIP_BUDGET = (2000, 512 * 1024 ** 2)+EMAIL = re.compile(rb"[\w.+-]+@[\w-]+(?:\.[\w-]+)+")   def safe_path(name):@@ -49,10 +54,15 @@ def role_for(name, data):             layer = next((p for p in parts if re.fullmatch(r"L[2-9][0-9]*", p)), None)             return "copper_inner_" + str(int(layer[1:]) - 1) if layer else "copper_inner"     if ext in ("drl", "xln", "exc", "txt") and re.search(r"(?m)^M48\s*$", text):-        stem = Path(name).stem.upper()-        if "NPTH" in stem or "NONPLATED" in stem or "NonPlated" in text:+        # The file's own X2 plating first (KiCad writes MixedPlating for a mixed file, whose+        # NonPlated aperture lines do not make it unplated), then whole words of the name.+        kind = match[1].split(",")[0] if match else None+        if kind in ("Plated", "NonPlated", "MixedPlating"):+            return {"Plated": "drill_plated", "NonPlated": "drill_unplated", "MixedPlating": "drill_mixed"}[kind]+        words = set(re.split(r"[^A-Z0-9]+", Path(name).stem.upper()))+        if words & {"NPTH", "NONPLATED"}:             return "drill_unplated"-        if "PTH" in stem or stem.endswith("-PLATED") or re.search(r"FileFunction,Plated,", text):+        if words & {"PTH", "PLATED"}:             return "drill_plated"         return "drill_mixed"     if re.fullmatch(r"g[1-9][0-9]*", ext):@@ -62,6 +72,7 @@ def role_for(name, data):  def read_zip(path):     out = {}+    total = 0     with zipfile.ZipFile(path) as z:         for item in z.infolist():             if item.is_dir():@@ -69,12 +80,64 @@ def read_zip(path):             safe_path(item.filename)             if item.filename in out:                 raise ValueError(f"duplicate zip entry: {item.filename}")+            total += item.file_size+            if len(out) >= ZIP_BUDGET[0] or total > ZIP_BUDGET[1]:+                raise ValueError("fabrication zip exceeds 2000 files or 512 MiB unpacked")             out[item.filename] = z.read(item)     return out  +def _drop_owner(o):+    if isinstance(o, dict):+        return {k: _drop_owner(v) for k, v in o.items() if k != "Owner"}+    if isinstance(o, list):+        return [_drop_owner(v) for v in o]+    return o+++def without_owner(files):+    """The files with every Gerber job file's Owner field removed; all other bytes unchanged.++    Fusion writes the EDA account's name and email there, and the zip is published with the page.+    A job file that is not readable JSON but holds an email address is refused.+    """+    out = {}+    for name, data in files.items():+        if name.lower().endswith(".gbrjob"):+            try:+                doc = json.loads(data)+            except ValueError:+                doc = None+            if doc is None and EMAIL.search(data):+                raise ValueError(f"{name} holds an email address and is not readable JSON; remove its owner or leave the file out")+            if doc is not None and _drop_owner(doc) != doc:+                data = (json.dumps(_drop_owner(doc), indent=2, ensure_ascii=False) + "\n").encode("utf-8")+        out[name] = data+    return out++ def layer_entries(files):     entries = [fingerprint(n, role_for(n, b), b) for n, b in sorted(files.items())]+    # .GKO is the board outline in EAGLE/Fusion CAM output but the keep-out layer in Altium's,+    # whose outline is a mechanical layer (.GM1). It counts as the outline only when nothing else is.+    if any(e["role"] == "outline" and not e["path"].lower().endswith(".gko") for e in entries):+        for e in entries:+            if e["role"] == "outline" and e["path"].lower().endswith(".gko"):+                e["role"] = "keepout"+    layers = [e for e in entries if e["role"] not in ("other", "gerber_job")]+    unread = [e["path"] for e in layers if not READER_EXT.search(e["path"])]+    if unread:+        raise ValueError("layer files the board reader skips by name (use KiCad or Protel extensions): " + ", ".join(unread))+    seen = {}+    for e in entries:+        if READER_EXT.search(e["path"]):+            seen.setdefault(PurePosixPath(e["path"]).name.lower(), []).append(e["path"])+    clash = [v for v in seen.values() if len(v) > 1]+    if clash:+        raise ValueError("files share a name, so the board reader cannot tell them apart: " + "; ".join(", ".join(v) for v in clash))+    for role in sorted(REQUIRED):+        if sum(e["role"] == role for e in entries) > 1:+            raise ValueError(f"more than one {role} layer: " + ", ".join(e["path"] for e in entries if e["role"] == role))     roles = {e["role"] for e in entries}     missing = REQUIRED - roles     if missing:@@ -84,7 +147,7 @@ def layer_entries(files):     # An empty mechanical-layer plot is not a board outline (Altium can emit one).     outlines = [files[e["path"]].decode("utf-8", errors="replace")                 for e in entries if e["role"] == "outline"]-    if not any(re.search(r"[XY][+-]?\d+[^*]*D0?1\*", t) for t in outlines):+    if not any(re.search(r"[XY][+-]?\d+[^*XY]*D0?1\*", t) for t in outlines):         raise ValueError("outline Gerber has no drawn segments; export the actual board boundary")     return entries @@ -133,18 +196,31 @@ def check(root, slug):     if len(zips) != 1 or zips[0]["path"] != f"{slug}-fab.zip":         raise ValueError(f"exactly one fab_zip named {slug}-fab.zip is required")     files = read_zip(root / zips[0]["path"])+    stripped = without_owner(files)+    owned = [n for n in sorted(files) if stripped[n] != files[n]]+    if owned:+        raise ValueError("Gerber job file names its owner (the EDA account's name and email); repack with create: " + ", ".join(owned))     actual = layer_entries(files)     expected = zips[0].get("entries", [])     by_name = {e["path"]: e for e in expected}     if len(by_name) != len(expected) or set(by_name) != set(files):         raise ValueError("zip entries and export.json disagree")+    # Bytes are the record; roles are recomputed here (layer_entries), so a record written by an+    # older classifier stays valid.     for entry in actual:         saved = by_name[entry["path"]]-        if any(saved.get(k) != entry[k] for k in ("sha256", "bytes", "role")):-            raise ValueError(f"zip entry changed or has wrong role: {entry['path']}")+        if any(saved.get(k) != entry[k] for k in ("sha256", "bytes")):+            raise ValueError(f"zip entry changed: {entry['path']}")     return rec  +def drill_plating(layers):+    roles = {e["role"] for e in layers}+    if "drill_mixed" in roles:+        return "mixed"+    return "split" if {"drill_plated", "drill_unplated"} <= roles else "plated" if "drill_plated" in roles else "unplated"++ def create(args):     root = Path(args.root).resolve()     if (root / "export.json").exists():@@ -161,12 +237,13 @@ def create(args):                 files[name] = p.read_bytes()     else:         files = read_zip(source)+    files = without_owner(files)     layers = layer_entries(files)     entries = [source_entry(root, s) for s in args.source]     target = root / f"{args.slug}-fab.zip"     if target.exists():         raise ValueError(f"output exists: {target}; use a fresh export folder")-    # Fixed ZIP metadata; the Gerber/drill bytes remain exactly as exported.+    # Fixed ZIP metadata; the Gerber/drill bytes remain exactly as exported (job files lose Owner).     with zipfile.ZipFile(target, "w", compression=zipfile.ZIP_DEFLATED) as z:         for name, data in sorted(files.items()):             item = zipfile.ZipInfo(name, date_time=(1980, 1, 1, 0, 0, 0))@@ -181,7 +258,7 @@ def create(args):            "exporter": args.exporter, "exporterVersion": args.exporter_version,            "design": args.slug, "files": entries, "stackup": stackup,            "gerbers": {"included": True},-           "drill": {"plating": "mixed" if any(e["role"] == "drill_mixed" for e in layers) else "split"}}+           "drill": {"plating": drill_plating(layers)}}     (root / "export.json").write_text(json.dumps(rec, indent=2) + "\n", encoding="utf-8")     return check(root, args.slug) 
tests/test_fab_bundle.py+58
@@ -80,6 +80,64 @@ class ExportTests(unittest.TestCase):         self.assertEqual(fab.role_for("MpinTest-Plated.TXT", b"M48\nMETRIC,TZ\n"), "drill_plated")         self.assertEqual(fab.role_for("inner.G2", b"G04 gerber*\n"), "copper_inner_2") +    def test_mixed_drill_by_x2_and_whole_words_of_the_name(self):+        mixed = b"M48\n; #@! TF.FileFunction,MixedPlating,1,2\n; #@! TA.AperFunction,NonPlated,NPTH,ComponentDrill\n"+        self.assertEqual(fab.role_for("BMV080_Molecule.drl", mixed), "drill_mixed")+        self.assertEqual(fab.role_for("depth.drl", b"M48\nMETRIC\n"), "drill_mixed")+        self.assertEqual(fab.role_for("board-NPTH.drl", b"M48\nMETRIC\n"), "drill_unplated")++    def test_plated_only_drill_is_not_called_split(self):+        rec = fab.create(self.args)+        self.assertEqual(rec["drill"]["plating"], "plated")++    def test_same_file_name_in_two_folders_refused(self):+        (self.layers / "old").mkdir()+        (self.layers / "old/board.gtl").write_bytes(b"G04 stale*\nX0Y0D02*\nX10Y10D01*\nM02*\n")+        with self.assertRaisesRegex(ValueError, "share a name"):+            fab.create(self.args)++    def test_layer_the_board_reader_skips_refused(self):+        (self.layers / "board.gtl").unlink()+        (self.layers / "top_copper.ger").write_bytes(b"%TF.FileFunction,Copper,L1,Top*%\nX0Y0D02*\nX10Y10D01*\nM02*\n")+        with self.assertRaisesRegex(ValueError, "reader skips"):+            fab.create(self.args)++    def test_altium_keepout_is_not_the_outline(self):+        (self.layers / "board.gm1").write_bytes(b"G04 Header*\nM02*\n")+        (self.layers / "board.gko").write_bytes(b"G04 keep-out*\nX0Y0D02*\nX10Y10D01*\nM02*\n")+        with self.assertRaisesRegex(ValueError, "no drawn segments"):+            fab.create(self.args)++    def test_fusion_style_nested_job_file_is_recorded_by_its_path(self):+        (self.layers / "Adom/Main/GerberJob").mkdir(parents=True)+        (self.layers / "Adom/Main/GerberJob/gerber_job.gbrjob").write_bytes(b'{"Header": {}}')+        rec = fab.create(self.args)+        entries = next(e for e in rec["files"] if e["role"] == "fab_zip")["entries"]+        self.assertIn("Adom/Main/GerberJob/gerber_job.gbrjob", {e["path"] for e in entries})++    def test_job_file_owner_is_removed_when_packing(self):+        job = {"Header": {"GenerationSoftware": {"Vendor": "Autodesk"}},+               "Overall": {"BoardThickness": 1.6, "Owner": "A Person <[email protected]>", "LayerNumber": 2}}+        (self.layers / "board.gbrjob").write_text(json.dumps(job))+        fab.create(self.args)+        packed = json.loads(fab.read_zip(self.root / "test-board-fab.zip")["board.gbrjob"])+        self.assertEqual(packed["Overall"], {"BoardThickness": 1.6, "LayerNumber": 2})+        fab.check(self.root, "test-board")++    def test_zip_whose_job_file_names_its_owner_is_refused(self):+        # A zip assembled by hand, with its record's zip hash updated to match.+        rec = fab.create(self.args)+        zpath = self.root / "test-board-fab.zip"+        files = fab.read_zip(zpath)+        files["board.gbrjob"] = json.dumps({"Overall": {"Owner": "A Person <[email protected]>"}}).encode()+        with zipfile.ZipFile(zpath, "w") as z:+            for n, b in files.items():+                z.writestr(n, b)+        rec["files"][-1].update(fab.fingerprint(zpath.name, "fab_zip", zpath.read_bytes()))+        (self.root / "export.json").write_text(json.dumps(rec))+        with self.assertRaisesRegex(ValueError, "names its owner"):+            fab.check(self.root, "test-board")+  if __name__ == "__main__":     unittest.main()