Merge remote-tracking branch 'origin/main' into codex/pr24-final

scripts/build-icons.py

@@ -218,7 +218,7 @@ def normalize_tabler(raw: str) -> str:
     body = re.sub(r'<path\s+stroke="none"[^/]*?/>', "", body)
     body = re.sub(r"\s+", " ", body).strip()
     return (
-        '<svg width="24" height="24" viewBox="0 0 24 24" fill="none" '
+        '<svg aria-hidden="true" width="24" height="24" viewBox="0 0 24 24" fill="none" '
         'stroke="currentColor" stroke-width="1.5" stroke-linecap="round" '
         f'stroke-linejoin="round">{body}</svg>'
     )
@@ -232,7 +232,7 @@ def normalize_simple(raw: str) -> str:
     body = inner.group(1).strip()
     body = re.sub(r"\s+", " ", body).strip()
     return (
-        '<svg width="24" height="24" viewBox="0 0 24 24" '
+        '<svg aria-hidden="true" width="24" height="24" viewBox="0 0 24 24" '
         f'fill="currentColor">{body}</svg>'
     )
 
@@ -284,7 +284,7 @@ def normalize_url(raw: str) -> str:
     body = re.sub(r'\bfill="none"', '', body)
     body = re.sub(r"\s+", " ", body).strip()
     return (
-        f'<svg width="24" height="24" viewBox="{viewbox}" '
+        f'<svg aria-hidden="true" width="24" height="24" viewBox="{viewbox}" '
         f'fill="currentColor">{body}</svg>'
     )
 
@@ -303,7 +303,7 @@ def normalize_devicon(raw: str) -> str:
     body = re.sub(r'\bclass="[^"]*"', 'fill="currentColor"', body)
     body = re.sub(r"\s+", " ", body).strip()
     return (
-        '<svg width="24" height="24" viewBox="0 0 128 128" '
+        '<svg aria-hidden="true" width="24" height="24" viewBox="0 0 128 128" '
         f'fill="currentColor">{body}</svg>'
     )
 
@@ -320,7 +320,7 @@ def normalize_logz(raw: str) -> str:
     body = re.sub(r'\bfill="#[0-9a-fA-F]{3,8}"', 'fill="currentColor"', body)
     body = re.sub(r"\s+", " ", body).strip()
     return (
-        '<svg width="24" height="24" viewBox="0 0 100 100" '
+        '<svg aria-hidden="true" width="24" height="24" viewBox="0 0 100 100" '
         f'fill="currentColor">{body}</svg>'
     )
 

scripts/lint-skin.py

@@ -8,6 +8,8 @@ skin. Use ``--all --baseline`` to skip those documented legacy files.
 import argparse
 import re
 import sys
+from dataclasses import dataclass, field
+from html.parser import HTMLParser
 from pathlib import Path
 
 
@@ -50,6 +52,244 @@ ALLOWED_FONTS = {
 CSS_FONT_KEYWORDS = {"inherit", "initial", "revert", "revert-layer", "unset"}
 
 
+@dataclass
+class SvgTextElement:
+    tag: str
+    element_id: str | None
+    line: int
+    offset: int
+    text: list[str] = field(default_factory=list)
+
+
+@dataclass
+class SvgElement:
+    attrs: dict[str, str | None]
+    line: int
+    offset: int
+    content_depth: int
+    first_child: str | None = None
+    ids: set[str] = field(default_factory=set)
+    bare_id_locations: list[tuple[int, int]] = field(default_factory=list)
+    titles: list[SvgTextElement] = field(default_factory=list)
+    descriptions: list[SvgTextElement] = field(default_factory=list)
+
+
+class AccessibleSvgParser(HTMLParser):
+    """Collect accessible-name data without rendering or executing HTML.
+
+    Trust boundary: input files are untrusted source text. ``HTMLParser`` only
+    tokenizes that text; this linter never evaluates scripts, fetches URLs, or
+    opens referenced resources.
+    """
+
+    def __init__(self, text):
+        super().__init__(convert_charrefs=True)
+        self.line_offsets = [0]
+        self.line_offsets.extend(
+            match.end() for match in re.finditer(r"\n", text)
+        )
+        self.element_stack = []
+        self.svg_stack = []
+        self.svgs = []
+        self.open_text = []
+        self.id_locations = {}
+
+    def source_offset(self):
+        line, column = self.getpos()
+        return self.line_offsets[line - 1] + column
+
+    def handle_starttag(self, tag, attrs):
+        tag = tag.casefold()
+        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
+            if element_id:
+                for svg in self.svg_stack:
+                    svg.ids.add(element_id)
+                    if element_id in {"title", "desc"}:
+                        svg.bare_id_locations.append((line, offset))
+
+        if tag == "svg":
+            svg = SvgElement(
+                attrs=attr_map,
+                line=line,
+                offset=offset,
+                content_depth=len(self.element_stack) + 1,
+            )
+            self.svgs.append(svg)
+            self.svg_stack.append(svg)
+        elif tag in {"title", "desc"} and self.svg_stack:
+            svg = self.svg_stack[-1]
+            if len(self.element_stack) == svg.content_depth:
+                element = SvgTextElement(
+                    tag=tag,
+                    element_id=attr_map.get("id"),
+                    line=line,
+                    offset=offset,
+                )
+                if tag == "title":
+                    svg.titles.append(element)
+                else:
+                    svg.descriptions.append(element)
+                self.open_text.append(element)
+
+        self.element_stack.append(tag)
+
+    def handle_startendtag(self, tag, attrs):
+        self.handle_starttag(tag, attrs)
+        self.handle_endtag(tag)
+
+    def handle_endtag(self, tag):
+        tag = tag.casefold()
+        for index in range(len(self.open_text) - 1, -1, -1):
+            if self.open_text[index].tag == tag:
+                del self.open_text[index]
+                break
+
+        if tag == "svg" and self.svg_stack:
+            self.svg_stack.pop()
+
+        for index in range(len(self.element_stack) - 1, -1, -1):
+            if self.element_stack[index] == tag:
+                del self.element_stack[index:]
+                break
+
+    def handle_data(self, data):
+        for element in self.open_text:
+            element.text.append(data)
+
+
+def lint_accessible_svgs(text, expected_slug):
+    parser = AccessibleSvgParser(text)
+    parser.feed(text)
+    parser.close()
+    findings = []
+
+    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":
+            continue
+
+        if (svg.attrs.get("role") or "").casefold() != "img":
+            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,
+                svg.offset,
+                "aria-labelledby must name the <title> and <desc>",
+            )
+        else:
+            missing_ids = [
+                element_id for element_id in labelled_by if element_id not in svg.ids
+            ]
+            if missing_ids:
+                add(
+                    svg.line,
+                    svg.offset,
+                    "aria-labelledby references missing id(s): "
+                    + ", ".join(missing_ids),
+                )
+
+        title = svg.titles[0] if svg.titles else None
+        description = svg.descriptions[0] if svg.descriptions else None
+        if title is None:
+            add(svg.line, svg.offset, "diagram <svg> must contain a <title>")
+        else:
+            if svg.first_child != "title":
+                add(
+                    title.line,
+                    title.offset,
+                    "<title> must be the first child element of <svg>",
+                )
+            if not "".join(title.text).strip():
+                add(title.line, title.offset, "<title> must not be empty")
+
+        if description is None:
+            add(svg.line, svg.offset, "diagram <svg> must contain a <desc>")
+        elif not "".join(description.text).strip():
+            add(description.line, description.offset, "<desc> must not be empty")
+
+        title_id = title.element_id if title else None
+        description_id = description.element_id if description else None
+        if svg.bare_id_locations:
+            line, offset = svg.bare_id_locations[0]
+            add(
+                line,
+                offset,
+                'bare id="title" and id="desc" are not allowed',
+            )
+
+        expected_title_id = f"{expected_slug}-title"
+        expected_description_id = f"{expected_slug}-desc"
+        if not placeholder_ids and (
+            title_id != expected_title_id or description_id != expected_description_id
+        ):
+            add(
+                svg.line,
+                svg.offset,
+                f'accessible-name IDs must match file slug "{expected_slug}": '
+                f'expected "{expected_title_id}" / "{expected_description_id}"',
+            )
+
+        if (
+            labelled_by
+            and (title_id not in labelled_by or description_id not in labelled_by)
+        ):
+            add(
+                svg.line,
+                svg.offset,
+                "aria-labelledby must name the <title> and <desc> IDs",
+            )
+
+    return findings
+
+
 def normalize_hex(value):
     value = value.lower()
     if len(value) == 4:
@@ -102,6 +342,10 @@ def display_path(path):
         return str(path)
 
 
+def diagram_slug(path):
+    return path.stem.removeprefix("example-")
+
+
 def named_families(value):
     families = []
     for raw_family in value.split(","):
@@ -114,8 +358,8 @@ def named_families(value):
     return families
 
 
-def lint_text(text, colors, rgb_triplets):
-    findings = []
+def lint_text(text, colors, rgb_triplets, expected_slug):
+    findings = lint_accessible_svgs(text, expected_slug)
 
     def add(offset, category, message):
         findings.append((line_number(text, offset), offset, category, message))
@@ -228,18 +472,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
 
@@ -247,7 +488,12 @@ 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)
+            expected_slug = diagram_slug(path)
+            if args.baseline and (path.name in baseline or relative in baseline):
+                findings = lint_accessible_svgs(text, expected_slug)
+            else:
+                findings = lint_text(text, colors, rgb_triplets, expected_slug)
         except OSError as error:
             findings = [(0, 0, "read-error", str(error))]
 

scripts/test-lint-a11y.py

@@ -0,0 +1,229 @@
+#!/usr/bin/env python3
+"""Adversarial tests for lint-skin.py's accessible-SVG contract."""
+
+from __future__ import annotations
+
+import importlib.util
+import shutil
+import subprocess
+import sys
+import tempfile
+from pathlib import Path
+
+
+ROOT = Path(__file__).resolve().parent.parent
+LINTER = ROOT / "scripts/lint-skin.py"
+BUILD_ICONS = ROOT / "scripts/build-icons.py"
+
+VALID_SVG = """\
+<svg xmlns="http://www.w3.org/2000/svg" role="img"
+     aria-labelledby="fixture-title fixture-desc">
+  <title id="fixture-title">Fixture diagram</title>
+  <desc id="fixture-desc">Diagram showing a fixture connected to a result.</desc>
+  <rect width="10" height="10"/>
+</svg>
+"""
+
+
+def run_linter(path: Path) -> subprocess.CompletedProcess[str]:
+    return subprocess.run(
+        [sys.executable, str(LINTER), str(path)],
+        capture_output=True,
+        text=True,
+        check=False,
+    )
+
+
+def require_failure(name: str, html: str, expected: str, directory: Path) -> None:
+    path = directory / f"{name}.html"
+    path.write_text(html, encoding="utf-8")
+    result = run_linter(path)
+    if result.returncode != 1:
+        raise AssertionError(
+            f"{name}: expected exit 1, got {result.returncode}\n{result.stdout}{result.stderr}"
+        )
+    if f"a11y: {expected}" not in result.stdout:
+        raise AssertionError(
+            f"{name}: missing expected a11y finding {expected!r}\n{result.stdout}"
+        )
+    print(f"OK: {name} rejected — {expected}")
+
+
+def require_pass(name: str, html: str, directory: Path) -> None:
+    path = directory / f"{name}.html"
+    path.write_text(html, encoding="utf-8")
+    result = run_linter(path)
+    if result.returncode != 0:
+        raise AssertionError(
+            f"{name}: expected exit 0, got {result.returncode}\n{result.stdout}{result.stderr}"
+        )
+    if "a11y:" in result.stdout:
+        raise AssertionError(f"{name}: unexpected a11y finding\n{result.stdout}")
+    print(f"OK: {name} accepted")
+
+
+def main() -> int:
+    with tempfile.TemporaryDirectory(prefix="lint-a11y-") as temp_dir:
+        directory = Path(temp_dir)
+
+        require_failure(
+            "missing-role",
+            VALID_SVG.replace(' role="img"', ""),
+            'diagram <svg> must carry role="img"',
+            directory,
+        )
+        require_failure(
+            "unresolved-labelled-by",
+            VALID_SVG.replace(
+                'aria-labelledby="fixture-title fixture-desc"',
+                'aria-labelledby="absent-title absent-desc"',
+            ),
+            "aria-labelledby references missing id(s): absent-title, absent-desc",
+            directory,
+        )
+        require_failure(
+            "labels-unrelated-elements",
+            VALID_SVG.replace(' id="fixture-title"', "")
+            .replace(' id="fixture-desc"', "")
+            .replace(
+                '  <rect width="10" height="10"/>',
+                '  <g id="fixture-title"></g>\n'
+                '  <g id="fixture-desc"></g>\n'
+                '  <rect width="10" height="10"/>',
+            ),
+            "aria-labelledby must name the <title> and <desc> IDs",
+            directory,
+        )
+        require_failure(
+            "late-title",
+            VALID_SVG.replace(
+                '  <title id="fixture-title">Fixture diagram</title>\n  <desc',
+                '  <defs></defs>\n  <title id="fixture-title">Fixture diagram</title>\n  <desc',
+            ),
+            "<title> must be the first child element of <svg>",
+            directory,
+        )
+        require_failure(
+            "empty-desc",
+            VALID_SVG.replace(
+                "Diagram showing a fixture connected to a result.", "  "
+            ),
+            "<desc> must not be empty",
+            directory,
+        )
+        require_failure(
+            "bare-ids",
+            VALID_SVG.replace("fixture-title", "title").replace(
+                "fixture-desc", "desc"
+            ),
+            'bare id="title" and id="desc" are not allowed',
+            directory,
+        )
+        require_failure(
+            "wrong-file-slug",
+            VALID_SVG,
+            'accessible-name IDs must match file slug "wrong-file-slug": '
+            'expected "wrong-file-slug-title" / "wrong-file-slug-desc"',
+            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">'
+            '<path d="M0 0h1v1z"/></svg>\n',
+            directory,
+        )
+
+        spec = importlib.util.spec_from_file_location("build_icons", BUILD_ICONS)
+        if spec is None or spec.loader is None:
+            raise AssertionError("generated-icons: could not load build-icons.py")
+        build_icons = importlib.util.module_from_spec(spec)
+        previous_bytecode_setting = sys.dont_write_bytecode
+        sys.dont_write_bytecode = True
+        try:
+            spec.loader.exec_module(build_icons)
+        finally:
+            sys.dont_write_bytecode = previous_bytecode_setting
+        generated_icons = {
+            "tabler": build_icons.normalize_tabler("<svg><path/></svg>"),
+            "simple": build_icons.normalize_simple("<svg><path/></svg>"),
+            "url": build_icons.normalize_url(
+                '<svg viewBox="0 0 24 24"><path/></svg>'
+            ),
+            "devicon": build_icons.normalize_devicon("<svg><path/></svg>"),
+            "logz": build_icons.normalize_logz("<svg><path/></svg>"),
+        }
+        for source, icon in generated_icons.items():
+            slug = f"generated-{source}-icon"
+            require_pass(
+                slug,
+                VALID_SVG.replace("fixture-title", f"{slug}-title")
+                .replace("fixture-desc", f"{slug}-desc")
+                .replace("</svg>", f"  {icon}\n</svg>"),
+                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
+
+
+if __name__ == "__main__":
+    sys.exit(main())