From d5c0075f75a5c7d7400b5d3793fbc96fd9131389 Mon Sep 17 00:00:00 2001
From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Date: Tue, 11 Aug 2026 16:51:56 -0700
Subject: [PATCH] fix: address self-review findings

---
 scripts/lint-skin.py      | 48 +++++++++++++++++++++++++++----
 scripts/test-lint-a11y.py | 60 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+), 6 deletions(-)

diff --git a/scripts/lint-skin.py b/scripts/lint-skin.py
index 1ca5449..3ff21c3 100644
--- a/scripts/lint-skin.py
+++ b/scripts/lint-skin.py
@@ -92,6 +92,7 @@ class AccessibleSvgParser(HTMLParser):
         self.svg_stack = []
         self.svgs = []
         self.open_text = []
+        self.id_locations = {}
 
     def source_offset(self):
         line, column = self.getpos()
@@ -102,13 +103,15 @@ class AccessibleSvgParser(HTMLParser):
         attr_map = {name.casefold(): value for name, value in attrs}
         offset = self.source_offset()
         line, _column = self.getpos()
+        element_id = attr_map.get("id")
+        if element_id:
+            self.id_locations.setdefault(element_id, []).append((line, offset))
 
         if self.svg_stack:
             parent = self.svg_stack[-1]
             if len(self.element_stack) == parent.content_depth:
                 if parent.first_child is None:
                     parent.first_child = tag
-            element_id = attr_map.get("id")
             if element_id:
                 for svg in self.svg_stack:
                     svg.ids.add(element_id)
@@ -174,6 +177,19 @@ def lint_accessible_svgs(text):
     def add(line, offset, message):
         findings.append((line, offset, "a11y", message))
 
+    naming_ids = {
+        element.element_id
+        for svg in parser.svgs
+        if (svg.attrs.get("aria-hidden") or "").casefold() != "true"
+        for element in svg.titles + svg.descriptions
+        if element.element_id
+    }
+    for element_id in sorted(naming_ids):
+        locations = parser.id_locations[element_id]
+        if len(locations) > 1:
+            line, offset = locations[1]
+            add(line, offset, f'duplicate accessible-name id="{element_id}" is not allowed')
+
     for svg in parser.svgs:
         aria_hidden = (svg.attrs.get("aria-hidden") or "").casefold()
         if aria_hidden == "true":
@@ -183,6 +199,25 @@ def lint_accessible_svgs(text):
             add(svg.line, svg.offset, 'diagram <svg> must carry role="img"')
 
         labelled_by = (svg.attrs.get("aria-labelledby") or "").split()
+        accessible_name_ids = labelled_by + [
+            element.element_id
+            for element in svg.titles + svg.descriptions
+            if element.element_id
+        ]
+        placeholder_ids = list(
+            dict.fromkeys(
+                element_id
+                for element_id in accessible_name_ids
+                if "[" in element_id or "]" in element_id
+            )
+        )
+        if placeholder_ids:
+            add(
+                svg.line,
+                svg.offset,
+                "accessible-name IDs contain unresolved placeholder(s): "
+                + ", ".join(placeholder_ids),
+            )
         if not labelled_by:
             add(
                 svg.line,
@@ -421,18 +456,15 @@ def main():
     colors, rgb_triplets = allowed_colors()
 
     skipped = 0
+    baseline = set()
     if args.all:
         paths = sorted(ASSET_DIR.glob("example-*.html"))
         if args.baseline:
             baseline = load_baseline()
-            selected = []
             for path in paths:
                 relative = display_path(path)
                 if path.name in baseline or relative in baseline:
                     skipped += 1
-                else:
-                    selected.append(path)
-            paths = selected
     else:
         paths = args.files
 
@@ -440,7 +472,11 @@ def main():
     for path in paths:
         try:
             text = path.read_text(encoding="utf-8")
-            findings = lint_text(text, colors, rgb_triplets)
+            relative = display_path(path)
+            if args.baseline and (path.name in baseline or relative in baseline):
+                findings = lint_accessible_svgs(text)
+            else:
+                findings = lint_text(text, colors, rgb_triplets)
         except OSError as error:
             findings = [(0, 0, "read-error", str(error))]
 
diff --git a/scripts/test-lint-a11y.py b/scripts/test-lint-a11y.py
index 53b7a40..19ea76c 100644
--- a/scripts/test-lint-a11y.py
+++ b/scripts/test-lint-a11y.py
@@ -3,6 +3,7 @@
 
 from __future__ import annotations
 
+import shutil
 import subprocess
 import sys
 import tempfile
@@ -116,6 +117,21 @@ def main() -> int:
             'bare id="title" and id="desc" are not allowed',
             directory,
         )
+        require_failure(
+            "placeholder-ids",
+            VALID_SVG.replace("fixture-title", "[diagram-slug]-title").replace(
+                "fixture-desc", "[diagram-slug]-desc"
+            ),
+            "accessible-name IDs contain unresolved placeholder(s): "
+            "[diagram-slug]-title, [diagram-slug]-desc",
+            directory,
+        )
+        require_failure(
+            "duplicate-naming-ids",
+            VALID_SVG + VALID_SVG,
+            'duplicate accessible-name id="fixture-desc" is not allowed',
+            directory,
+        )
         require_pass(
             "decorative",
             '<svg xmlns="http://www.w3.org/2000/svg" aria-hidden="true">'
@@ -123,6 +139,50 @@ def main() -> int:
             directory,
         )
 
+        project = directory / "baseline-project"
+        (project / "scripts").mkdir(parents=True)
+        (project / "skills/diagram-design/assets").mkdir(parents=True)
+        (project / "skills/diagram-design/references").mkdir(parents=True)
+        shutil.copy2(LINTER, project / "scripts/lint-skin.py")
+        shutil.copy2(
+            ROOT / "skills/diagram-design/references/style-guide.md",
+            project / "skills/diagram-design/references/style-guide.md",
+        )
+        (project / "scripts/lint-skin-baseline.txt").write_text(
+            "example-baseline.html\n", encoding="utf-8"
+        )
+        (project / "skills/diagram-design/assets/example-baseline.html").write_text(
+            '<svg style="color: #123456"></svg>\n', encoding="utf-8"
+        )
+        baseline_result = subprocess.run(
+            [
+                sys.executable,
+                str(project / "scripts/lint-skin.py"),
+                "--all",
+                "--baseline",
+            ],
+            capture_output=True,
+            text=True,
+            check=False,
+        )
+        if baseline_result.returncode != 1:
+            raise AssertionError(
+                "baseline-a11y: expected exit 1, got "
+                f"{baseline_result.returncode}\n"
+                f"{baseline_result.stdout}{baseline_result.stderr}"
+            )
+        if "a11y: diagram <svg> must carry role=\"img\"" not in baseline_result.stdout:
+            raise AssertionError(
+                "baseline-a11y: missing expected accessibility finding\n"
+                f"{baseline_result.stdout}"
+            )
+        if "color:" in baseline_result.stdout:
+            raise AssertionError(
+                "baseline-a11y: legacy visual finding was not exempted\n"
+                f"{baseline_result.stdout}"
+            )
+        print("OK: baseline file receives a11y checks but keeps visual exemptions")
+
     print("All accessible-SVG lint cases passed.")
     return 0