fix(treemap): close post-merge fail-open gaps (#127)
* fix(treemap): close fail-open metadata gaps * chore: bump plugin version to 2.5.18 * fix(treemap): validate declared cell geometry * fix(package): validate YAML escape sequences * fix(package): bound Unicode YAML escapes
This commit is contained in:
@@ -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"
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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"))
|
||||
|
||||
@@ -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(
|
||||
'<rect x="940" y="296" width="16" height="124"',
|
||||
'<rect x="940" y="296" width="2" height="124"',
|
||||
).replace('cx="948" cy="320"', 'cx="941" cy="320"').replace(
|
||||
'<text x="948" y="323"', '<text x="941" y="323"'
|
||||
)
|
||||
if below_threshold == source:
|
||||
failures.append("could not build the below-threshold-cell fixture")
|
||||
else:
|
||||
sliver = next(
|
||||
(cell for cell in PARSE_CELLS(below_threshold) if cell.w == 2 and cell.h == 124),
|
||||
None,
|
||||
)
|
||||
if sliver is None or sliver.rect_count != 2:
|
||||
failures.append(
|
||||
"the below-threshold metadata cell lost its same-geometry mask/body association"
|
||||
)
|
||||
code, output = run(write(directory, "below-threshold-cell.html", below_threshold))
|
||||
if code == 0:
|
||||
failures.append("a metadata-bearing cell below CELL_MIN_AREA was discarded")
|
||||
elif "cell 2x124" not in output:
|
||||
failures.append(
|
||||
f"below-threshold cell was not preserved as a logical cell: {output.strip()}"
|
||||
)
|
||||
else:
|
||||
print("OK: metadata-bearing cells bypass the decorative-area threshold")
|
||||
|
||||
no_rounding_marker = source.replace(
|
||||
'rx="2" data-share="0.56"', 'rx="3" data-share="0.56"', 1
|
||||
)
|
||||
code, output = run(write(directory, "metadata-with-nonstandard-rx.html", no_rounding_marker))
|
||||
if code != 0:
|
||||
failures.append(
|
||||
f"an otherwise valid data-share rect depended on decorative rx=2: {output.strip()}"
|
||||
)
|
||||
else:
|
||||
print("OK: data-share identifies a cell independently of decorative rx")
|
||||
|
||||
# 14. Every percentage claim hosted by a cell must agree. Reading only
|
||||
# the first match accepted a later contradiction when 59% appeared
|
||||
# before 42% in the same cell.
|
||||
conflicting_labels = source.replace(
|
||||
"4.78B · 59% of world",
|
||||
"4.78B · 59% of world · alternate claim 42%",
|
||||
1,
|
||||
)
|
||||
code, output = run(write(directory, "conflicting-label-claims.html", conflicting_labels))
|
||||
if code == 0:
|
||||
failures.append("distinct percentage claims in one cell were accepted")
|
||||
elif "conflicting percentage claims" not in output:
|
||||
failures.append(f"conflicting labels lacked the right finding: {output.strip()}")
|
||||
else:
|
||||
print("OK: every hosted percentage claim must agree")
|
||||
|
||||
# 15. Explicit metadata outranks both sides of the decorative-rect
|
||||
# heuristic. A declared cell cannot disappear merely because a bad
|
||||
# edit makes it implausibly wide or tall.
|
||||
for label, old, new, dimensions in (
|
||||
(
|
||||
"above-max-width",
|
||||
'<rect x="940" y="296" width="16" height="124"',
|
||||
'<rect x="40" y="296" width="901" height="124"',
|
||||
"901x124",
|
||||
),
|
||||
(
|
||||
"above-max-height",
|
||||
'<rect x="940" y="296" width="16" height="124"',
|
||||
'<rect x="940" y="40" width="16" height="461"',
|
||||
"16x461",
|
||||
),
|
||||
):
|
||||
oversized_declared = source.replace(old, new)
|
||||
if oversized_declared == source:
|
||||
failures.append(f"could not build the {label} fixture")
|
||||
continue
|
||||
code, output = run(write(directory, f"{label}.html", oversized_declared))
|
||||
if code == 0:
|
||||
failures.append(f"metadata-bearing {label} cell was discarded")
|
||||
elif f"cell {dimensions}" not in output:
|
||||
failures.append(f"{label} cell was not retained: {output.strip()}")
|
||||
else:
|
||||
print(f"OK: metadata-bearing {label} cells bypass decorative limits")
|
||||
|
||||
# 16. Geometry on a declared cell is untrusted metadata too. Missing,
|
||||
# malformed, non-finite, zero, or negative dimensions must be a
|
||||
# finding and must never poison the area total with NaN/Infinity.
|
||||
asia_declared = (
|
||||
'<rect x="40" y="40" width="532" height="380" rx="2" '
|
||||
'data-share="59.04"'
|
||||
)
|
||||
for label, replacement in (
|
||||
("missing", '<rect x="40" y="40" height="380" rx="2" data-share="59.04"'),
|
||||
("nonnumeric", asia_declared.replace('width="532"', 'width="wide"')),
|
||||
("nan", asia_declared.replace('width="532"', 'width="NaN"')),
|
||||
("infinity", asia_declared.replace('width="532"', 'width="Infinity"')),
|
||||
("zero", asia_declared.replace('width="532"', 'width="0"')),
|
||||
("negative", asia_declared.replace('width="532"', 'width="-1"')),
|
||||
):
|
||||
bad_geometry = source.replace(asia_declared, replacement, 1)
|
||||
if bad_geometry == source:
|
||||
failures.append(f"could not build the {label}-geometry fixture")
|
||||
continue
|
||||
code, output = run(write(directory, f"geometry-{label}.html", bad_geometry))
|
||||
if code == 0:
|
||||
failures.append(f"{label} data-share geometry was accepted")
|
||||
elif "data-share rect" not in output:
|
||||
failures.append(f"{label} geometry lacked an explicit finding: {output.strip()}")
|
||||
else:
|
||||
print(f"OK: {label} data-share geometry fails closed")
|
||||
|
||||
for failure in failures:
|
||||
print(f"FAIL: {failure}")
|
||||
if failures:
|
||||
|
||||
@@ -280,13 +280,43 @@ def verify_codex_skill_path(root: Path, codex_manifest: dict[str, Any], errors:
|
||||
errors.append(f"Codex skills path does not contain {PLUGIN_NAME}/SKILL.md")
|
||||
|
||||
|
||||
def valid_double_quoted_yaml_scalar(value: str) -> bool:
|
||||
"""Validate escapes in the single-line double-quoted YAML subset we accept."""
|
||||
match = re.fullmatch(r'"(?P<body>(?:[^"\\]|\\.)*)"(?:\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<indent> *)(?P<key>[A-Za-z_][\w.-]*):(?P<value>(?: +.*)?)",
|
||||
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:
|
||||
|
||||
+67
-14
@@ -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'<rect\b(?P<attrs>[^>]*\brx="2"[^>]*?)/?>', re.IGNORECASE)
|
||||
CELL_RE = re.compile(r"<rect\b(?P<attrs>[^>]*)/?>", re.IGNORECASE)
|
||||
TEXT_RE = re.compile(r"<text\b(?P<attrs>[^>]*)>(?P<body>.*?)</text>", re.IGNORECASE | re.DOTALL)
|
||||
CIRCLE_RE = re.compile(r"<circle\b(?P<attrs>[^>]*)/?>", re.IGNORECASE)
|
||||
ATTR_RE = re.compile(r'(?P<name>[\w:-]+)="(?P<value>[^"]*)"')
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user