diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index aefef2d..768c7a8 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "diagram-design", "description": "Create branded architecture, IT current-state, flowchart, sequence, state machine, ER/data model, timeline, swimlane, quadrant, radar/spider, loop/flywheel, nested, tree, org chart, layer stack, Venn, pyramid/funnel, treemap, bar, line, Gantt and scatter charts, high-level, process, medallion, data flow, DP integration, DP security matrix, sankey, fishbone, Wardley map, kanban, user journey, deployment, dependency graph, UML class, story map, or database schema diagrams as standalone HTML/SVG/PNG. Redraw .drawio/.drawio.png/.drawio.svg or Mermaid .mmd sources at a chosen size/detail; onboard brand tokens from a website; add semantic patterns, callouts, accessible motion, or sketchy/hand-drawn styling.", - "version": "2.5.17", + "version": "2.5.18", "author": { "name": "Cathryn Lavery", "url": "https://github.com/cathrynlavery" diff --git a/.codex-plugin/plugin.json b/.codex-plugin/plugin.json index 7a7fb4d..8c2024e 100644 --- a/.codex-plugin/plugin.json +++ b/.codex-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "diagram-design", "description": "Create branded architecture, IT current-state, flowchart, sequence, state machine, ER/data model, timeline, swimlane, quadrant, radar/spider, loop/flywheel, nested, tree, org chart, layer stack, Venn, pyramid/funnel, treemap, bar, line, Gantt and scatter charts, high-level, process, medallion, data flow, DP integration, DP security matrix, sankey, fishbone, Wardley map, kanban, user journey, deployment, dependency graph, UML class, story map, or database schema diagrams as standalone HTML/SVG/PNG. Redraw .drawio/.drawio.png/.drawio.svg or Mermaid .mmd sources at a chosen size/detail; onboard brand tokens from a website; add semantic patterns, callouts, accessible motion, or sketchy/hand-drawn styling.", - "version": "2.5.17", + "version": "2.5.18", "author": { "name": "Cathryn Lavery", "url": "https://github.com/cathrynlavery" diff --git a/.factory-plugin/plugin.json b/.factory-plugin/plugin.json index aefef2d..768c7a8 100644 --- a/.factory-plugin/plugin.json +++ b/.factory-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "diagram-design", "description": "Create branded architecture, IT current-state, flowchart, sequence, state machine, ER/data model, timeline, swimlane, quadrant, radar/spider, loop/flywheel, nested, tree, org chart, layer stack, Venn, pyramid/funnel, treemap, bar, line, Gantt and scatter charts, high-level, process, medallion, data flow, DP integration, DP security matrix, sankey, fishbone, Wardley map, kanban, user journey, deployment, dependency graph, UML class, story map, or database schema diagrams as standalone HTML/SVG/PNG. Redraw .drawio/.drawio.png/.drawio.svg or Mermaid .mmd sources at a chosen size/detail; onboard brand tokens from a website; add semantic patterns, callouts, accessible motion, or sketchy/hand-drawn styling.", - "version": "2.5.17", + "version": "2.5.18", "author": { "name": "Cathryn Lavery", "url": "https://github.com/cathrynlavery" diff --git a/scripts/test-plugin-package.py b/scripts/test-plugin-package.py index 3e3f0c1..9dc7e1a 100755 --- a/scripts/test-plugin-package.py +++ b/scripts/test-plugin-package.py @@ -305,6 +305,129 @@ def test_verifier() -> None: "frontmatter has no metadata.version", ) + with package_repo() as root: + set_versions(root, "1.2.4", "1.2.4") + skill = root / "skills" / PLUGIN_NAME / "SKILL.md" + skill.write_text( + f'---\nname: {PLUGIN_NAME}\nmetadata:\n version: [broken\n' + ' version: "1.2"\n---\n', + encoding="utf-8", + ) + expect_failure( + "malformed version plus valid duplicate", + VERIFY.verify_package(root, "HEAD"), + "frontmatter has no metadata.version", + ) + + with package_repo() as root: + set_versions(root, "1.2.4", "1.2.4") + skill = root / "skills" / PLUGIN_NAME / "SKILL.md" + skill.write_text( + f'---\nname: {PLUGIN_NAME}\nmetadata:\n version: "1.2"\n' + ' version: "1.2"\n---\n', + encoding="utf-8", + ) + expect_failure( + "duplicate valid metadata versions", + VERIFY.verify_package(root, "HEAD"), + "frontmatter has no metadata.version", + ) + + with package_repo() as root: + set_versions(root, "1.2.4", "1.2.4") + skill = root / "skills" / PLUGIN_NAME / "SKILL.md" + skill.write_text( + f'---\nname: {PLUGIN_NAME}\ndescription: [unterminated\n' + 'metadata:\n version: "1.2"\n---\n', + encoding="utf-8", + ) + expect_failure( + "malformed surrounding frontmatter", + VERIFY.verify_package(root, "HEAD"), + "frontmatter has no metadata.version", + ) + + with package_repo() as root: + set_versions(root, "1.2.4", "1.2.4") + skill = root / "skills" / PLUGIN_NAME / "SKILL.md" + skill.write_text( + f'---\nname: {PLUGIN_NAME}\nmetadata:\n version:"1.2"\n---\n', + encoding="utf-8", + ) + expect_failure( + "version value without YAML separation", + VERIFY.verify_package(root, "HEAD"), + "frontmatter has no metadata.version", + ) + + with package_repo() as root: + set_versions(root, "1.2.4", "1.2.4") + skill = root / "skills" / PLUGIN_NAME / "SKILL.md" + skill.write_text( + f'---\nname: {PLUGIN_NAME}\ndescription: broken: value\n' + 'metadata:\n version: "1.2"\n---\n', + encoding="utf-8", + ) + expect_failure( + "unquoted colon in frontmatter scalar", + VERIFY.verify_package(root, "HEAD"), + "frontmatter has no metadata.version", + ) + + with package_repo() as root: + set_versions(root, "1.2.4", "1.2.4") + skill = root / "skills" / PLUGIN_NAME / "SKILL.md" + skill.write_text( + f'---\nname: {PLUGIN_NAME}\ndescription: "bad\\q"\n' + 'metadata:\n version: "1.2"\n---\n', + encoding="utf-8", + ) + expect_failure( + "invalid double-quoted YAML escape", + VERIFY.verify_package(root, "HEAD"), + "frontmatter has no metadata.version", + ) + + with package_repo() as root: + set_versions(root, "1.2.4", "1.2.4") + skill = root / "skills" / PLUGIN_NAME / "SKILL.md" + skill.write_text( + f'---\nname: {PLUGIN_NAME}\ndescription: "line one\\nline two and ' + '\\"quoted\\" with unicode \\u2192"\nmetadata:\n version: "1.2"\n---\n', + encoding="utf-8", + ) + errors = VERIFY.verify_package(root, "HEAD") + if errors: + raise AssertionError(f"valid escaped YAML scalar failed: {errors}") + print("OK: valid double-quoted YAML escapes accepted") + + with package_repo() as root: + set_versions(root, "1.2.4", "1.2.4") + skill = root / "skills" / PLUGIN_NAME / "SKILL.md" + skill.write_text( + f'---\nname: {PLUGIN_NAME}\ndescription: "highest scalar \\U0010FFFF"\n' + 'metadata:\n version: "1.2"\n---\n', + encoding="utf-8", + ) + errors = VERIFY.verify_package(root, "HEAD") + if errors: + raise AssertionError(f"maximum Unicode YAML escape failed: {errors}") + print("OK: maximum Unicode YAML escape U+10FFFF accepted") + + with package_repo() as root: + set_versions(root, "1.2.4", "1.2.4") + skill = root / "skills" / PLUGIN_NAME / "SKILL.md" + skill.write_text( + f'---\nname: {PLUGIN_NAME}\ndescription: "out of range \\U00110000"\n' + 'metadata:\n version: "1.2"\n---\n', + encoding="utf-8", + ) + expect_failure( + "out-of-range Unicode YAML escape", + VERIFY.verify_package(root, "HEAD"), + "frontmatter has no metadata.version", + ) + def test_bumper() -> None: cases = (("patch", "1.2.4"), ("minor", "1.3.0"), ("major", "2.0.0")) diff --git a/scripts/test-verify-treemap.py b/scripts/test-verify-treemap.py index 064a972..60a3c36 100644 --- a/scripts/test-verify-treemap.py +++ b/scripts/test-verify-treemap.py @@ -13,6 +13,7 @@ Exit: 0 all pass, 1 a case failed. from __future__ import annotations import runpy +import re import subprocess import sys import tempfile @@ -22,9 +23,9 @@ ROOT = Path(__file__).resolve().parent.parent CHECKER = ROOT / "scripts/verify-treemap.py" GOOD = ROOT / "skills/diagram-design/assets/example-treemap.html" SHIPPED = sorted(GOOD.parent.glob("example-treemap*.html")) -ESTIMATED_ADVANCE = runpy.run_path(str(CHECKER), run_name="verify_treemap_test")[ - "estimated_advance" -] +VERIFY_NAMESPACE = runpy.run_path(str(CHECKER), run_name="verify_treemap_test") +ESTIMATED_ADVANCE = VERIFY_NAMESPACE["estimated_advance"] +PARSE_CELLS = VERIFY_NAMESPACE["parse_cells"] KOREAN_LABEL = "남아메리카 인구 구성 비율 요약" JAPANESE_LABEL = "南アメリカ人口構成比率の概要" @@ -388,6 +389,7 @@ def main() -> int: # 12. A treemap with too few parseable cells is unchecked, not clean. unparsable = source.replace(' rx="2"', ' rx="3"') + unparsable = re.sub(r' data-share="[^"]+"', "", unparsable) code, output = run(write(directory, "too-few-cells.html", unparsable)) if code == 0: failures.append("a treemap with no parseable cells was accepted") @@ -396,6 +398,120 @@ def main() -> int: else: print("OK: too few parseable cells fails closed") + # 13. An explicitly metadata-bearing cell remains a cell even below + # the generic 400px decorative-rect threshold. Oceania at 2x124 + # used to disappear with its mask, leaving a plausible 99.43% + # total that slipped through the rounding tolerance. + below_threshold = source.replace( + ' bool: + """Validate escapes in the single-line double-quoted YAML subset we accept.""" + match = re.fullmatch(r'"(?P(?:[^"\\]|\\.)*)"(?:\s+#.*)?', value) + if match is None: + return False + body = match.group("body") + simple_escapes = set('0abtnvfre "\\/N_LP') + index = 0 + while index < len(body): + if body[index] != "\\": + index += 1 + continue + index += 1 + escape = body[index] + if escape in simple_escapes: + index += 1 + continue + digits = {"x": 2, "u": 4, "U": 8}.get(escape) + if digits is None: + return False + encoded = body[index + 1 : index + 1 + digits] + if len(encoded) != digits or re.fullmatch(r"[0-9A-Fa-f]+", encoded) is None: + return False + if int(encoded, 16) > 0x10FFFF: + return False + index += 1 + digits + return True + + def parse_frontmatter_metadata_version(text: str) -> str | None: """Return exactly one direct ``metadata.version`` from YAML frontmatter. This intentionally does not search the Markdown body: examples and prose are allowed to mention ``version:``, but they cannot validate package - metadata. The small parser is limited to the mapping shape this skill owns - and rejects duplicate or nested version keys instead of guessing. + metadata. The dependency-free parser accepts the mapping subset this + skill's frontmatter uses and fails closed on malformed structure, duplicate + keys, collection syntax, or malformed scalar values instead of guessing. """ lines = text.splitlines() if not lines or lines[0].strip() != "---": @@ -298,38 +328,83 @@ def parse_frontmatter_metadata_version(text: str) -> str | None: except StopIteration: return None - frontmatter = lines[1:closing] - metadata_rows = [ - index - for index, line in enumerate(frontmatter) - if re.fullmatch(r"metadata:\s*(?:#.*)?", line) - ] - if len(metadata_rows) != 1: - return None + entries: list[tuple[tuple[str, ...], str, str]] = [] + open_mappings: list[tuple[int, str]] = [] + seen_keys: dict[tuple[str, ...], set[str]] = {} + previous_indent = -1 + previous_opened_mapping = False - start = metadata_rows[0] + 1 - block: list[str] = [] - for line in frontmatter[start:]: - if line and not line[0].isspace(): - break - block.append(line) - - content = [line for line in block if line.strip() and not line.lstrip().startswith("#")] - if not content: - return None - direct_indent = min(len(line) - len(line.lstrip()) for line in content) - versions: list[str] = [] - for line in content: - indent = len(line) - len(line.lstrip()) - if indent != direct_indent: + for raw_line in lines[1:closing]: + if not raw_line.strip() or raw_line.lstrip().startswith("#"): continue + if "\t" in raw_line: + return None match = re.fullmatch( - r"\s*version:\s*(?:\"([^\"]+)\"|'([^']+)'|([0-9]+(?:\.[0-9]+)*))\s*(?:#.*)?", - line, + r"(?P *)(?P[A-Za-z_][\w.-]*):(?P(?: +.*)?)", + raw_line, ) - if match is not None: - versions.append(next(value for value in match.groups() if value is not None)) - return versions[0] if len(versions) == 1 else None + if match is None: + return None + + indent = len(match.group("indent")) + if previous_indent >= 0 and indent > previous_indent and not previous_opened_mapping: + return None + while open_mappings and indent <= open_mappings[-1][0]: + open_mappings.pop() + if indent > 0 and not open_mappings: + return None + + parent = tuple(key for _, key in open_mappings) + key = match.group("key") + value = match.group("value").strip() + duplicate = key in seen_keys.setdefault(parent, set()) + seen_keys[parent].add(key) + # Count metadata.version rows below even when one value is malformed; + # all other duplicate mapping keys are immediately ambiguous. + if duplicate and not (parent == ("metadata",) and key == "version"): + return None + + opens_mapping = not value or value.startswith("#") + is_metadata_version = parent == ("metadata",) and key == "version" + if not opens_mapping and not is_metadata_version: + # Flow collections and block scalars are not used by this package's + # frontmatter. Rejecting them keeps malformed brackets or multiline + # YAML from being mistaken for valid metadata without a YAML runtime. + if value[0] in "[{|>" or any(char in value for char in "[]{}"): + return None + if value[0] == '"': + if not valid_double_quoted_yaml_scalar(value): + return None + elif value[0] == "'": + if re.fullmatch(r"'[^']*'(?:\s+#.*)?", value) is None: + return None + elif ": " in value: + return None + + entries.append((parent, key, value)) + if opens_mapping: + open_mappings.append((indent, key)) + previous_indent = indent + previous_opened_mapping = opens_mapping + + metadata_rows = [entry for entry in entries if entry[0] == () and entry[1] == "metadata"] + if len(metadata_rows) != 1 or metadata_rows[0][2]: + return None + + version_rows = [ + value + for parent, key, value in entries + if parent == ("metadata",) and key == "version" + ] + if len(version_rows) != 1: + return None + match = re.fullmatch( + r"(?:\"([^\"]+)\"|'([^']+)'|([0-9]+(?:\.[0-9]+)*))\s*(?:#.*)?", + version_rows[0], + ) + if match is None: + return None + return next(value for value in match.groups() if value is not None) def verify_skill_metadata_version(root: Path, manifests: dict[str, dict[str, Any]], errors: list[str]) -> None: diff --git a/scripts/verify-treemap.py b/scripts/verify-treemap.py index 33b0e04..65a4cee 100644 --- a/scripts/verify-treemap.py +++ b/scripts/verify-treemap.py @@ -47,7 +47,7 @@ from pathlib import Path ROOT = Path(__file__).resolve().parent.parent ASSET_DIR = ROOT / "skills/diagram-design/assets" -CELL_RE = re.compile(r'[^>]*\brx="2"[^>]*?)/?>', re.IGNORECASE) +CELL_RE = re.compile(r"[^>]*)/?>", re.IGNORECASE) TEXT_RE = re.compile(r"[^>]*)>(?P.*?)", re.IGNORECASE | re.DOTALL) CIRCLE_RE = re.compile(r"[^>]*)/?>", re.IGNORECASE) ATTR_RE = re.compile(r'(?P[\w:-]+)="(?P[^"]*)"') @@ -147,7 +147,7 @@ def estimated_advance(text: str, mono: bool) -> float: return advance -def parse_cells(source: str) -> list[Box]: +def parse_cells(source: str, findings: list[str] | None = None) -> list[Box]: """Deduped cell rects. Each cell is painted twice: paper mask, then body. `data-share` may sit on either rect of the pair, so the declared share is @@ -156,6 +156,9 @@ def parse_cells(source: str) -> list[Box]: seen: list[Box] = [] for m in CELL_RE.finditer(source): attrs = {a.group("name"): a.group("value") for a in ATTR_RE.finditer(m.group("attrs"))} + has_share_metadata = "data-share" in attrs + if attrs.get("rx") != "2" and not has_share_metadata: + continue try: box = Box( float(attrs["x"]), @@ -164,7 +167,23 @@ def parse_cells(source: str) -> list[Box]: float(attrs["height"]), m.start(), ) - except (KeyError, ValueError): + except (KeyError, ValueError) as exc: + if has_share_metadata and findings is not None: + findings.append( + f"line {line_of(source, m.start())}: data-share rect has missing or " + f"unparseable x/y/width/height geometry ({exc})" + ) + continue + coordinates = (box.x, box.y, box.w, box.h) + if has_share_metadata and ( + not all(math.isfinite(value) for value in coordinates) or box.w <= 0 or box.h <= 0 + ): + if findings is not None: + findings.append( + f"line {line_of(source, m.start())}: data-share rect geometry must have " + "finite x/y/width/height and strictly positive width/height; " + f"got x={box.x:g}, y={box.y:g}, width={box.w:g}, height={box.h:g}" + ) continue if "data-share" in attrs: raw_share = attrs["data-share"] @@ -181,8 +200,6 @@ def parse_cells(source: str) -> list[Box]: ) else: box.share = parsed_share - if box.area < CELL_MIN_AREA or box.w > CELL_MAX_W or box.h > CELL_MAX_H: - continue twin = next( ( s @@ -208,7 +225,13 @@ def parse_cells(source: str) -> list[Box]: twin.share = box.share continue seen.append(box) - return seen + return [ + box + for box in seen + if box.share is not None + or box.share_errors + or (box.area >= CELL_MIN_AREA and box.w <= CELL_MAX_W and box.h <= CELL_MAX_H) + ] def label_box(attrs: dict[str, str], body: str) -> Box | None: @@ -271,14 +294,17 @@ def parse_claim(text: str) -> tuple[float | None, float | None]: def check(path: Path) -> list[str]: source = path.read_text(encoding="utf-8") - cells = parse_cells(source) + findings: list[str] = [] + parse_findings: list[str] = [] + cells = parse_cells(source, parse_findings) + findings.extend(f"{path.name}:{finding}" for finding in parse_findings) if len(cells) < 3: - return [ + findings.append( f"{path.name}: expected at least 3 parseable treemap cells, found {len(cells)} — " "the area contract cannot be verified" - ] + ) + return findings - findings: list[str] = [] labels: dict[int, list[str]] = {index: [] for index in range(len(cells))} for m in TEXT_RE.finditer(source): @@ -388,6 +414,11 @@ def check(path: Path) -> list[str]: return findings drawn_total = sum(cell.area for cell in cells) + if not math.isfinite(drawn_total) or drawn_total <= 0: + findings.append( + f"{path.name}: parsed cell area total must be finite and positive; got {drawn_total:g}" + ) + return findings if any(cell.share_errors for cell in cells): return findings @@ -410,12 +441,34 @@ def check(path: Path) -> list[str]: f"draws {area_share:.2f}% of the area but declares {declared:.2f}% — " f"{relative:+.1f}% relative. Area is the only encoding; resize the cell" ) + + # A label and the metadata are two statements of one fact. Read every + # percentage in the hosted labels: search() alone silently accepted a + # later contradictory claim when the first happened to be correct. + label_text = " ".join(labels[index]) + percentages = [ + value + for match in PCT_RE.finditer(label_text) + if (value := _number(match)) is not None + ] + distinct_percentages: list[float] = [] + for percentage in percentages: + if not any( + math.isclose(percentage, known, rel_tol=0.0, abs_tol=1e-9) + for known in distinct_percentages + ): + distinct_percentages.append(percentage) + if len(distinct_percentages) > 1: + rendered = ", ".join(f"{value:g}%" for value in distinct_percentages) + findings.append( + f"{path.name}:{line_of(source, cell.offset)}: cell {cell.w:g}x{cell.h:g} " + f"has conflicting percentage claims ({rendered}) — every hosted label " + "must state the same share" + ) continue - # A label and the metadata are two statements of one fact. Let them - # disagree and the picture lies while the gate stays green. - percentage, _ = parse_claim(" ".join(labels[index])) - if percentage is not None and declared > 0: + if distinct_percentages and declared > 0: + percentage = distinct_percentages[0] drift = abs(percentage - declared) # 1pp covers honest rounding of a displayed integer percentage. if drift > 1.0: