fix(treemap): verify every cell share (#124)
* fix(treemap): verify every cell's area, including cells too small to label Closes #89. The area check derived its basis from in-cell labels, so a cell with no label was excluded from it. That is precisely the sliver -- the cell a 4px grid distorts most, and the one that shipped 24% undersized during review of #87. On current main, widening Oceania from 16px to 24px, a 50% area distortion, still reports OK. Every cell now declares `data-share`, its percentage of the whole, and the verifier checks all of them against it. Cells without it are reported rather than skipped: a treemap whose cells cannot be parsed is not a passing treemap, it is an unchecked one, and answering OK to it is the defect. The label and the metadata are also cross-checked, because two statements of one fact is a new way for the picture to lie while the gate stays green. Adversarial tests gain the oversized and undersized unlabelled sliver (both pass on current main and fail here), a fail-closed case for missing metadata, and a label/metadata divergence case. SKILL.md `metadata.version` said 2.4 against 2.5.0 manifests. The manifests are bumped by a script and that field is hand-maintained, so nothing read both numbers and nothing could notice. verify-plugin-package now asserts it tracks the manifest MAJOR.MINOR. The package test's fixture gains the field with it -- a SKILL.md without one is not a valid package, and a fixture missing it would only prove the gate can be tripped by its own test data. The examples regained the `PARTS ROUNDED, MAY NOT SUM` disclosure the type reference requires: displayed parts round up to 8.10B against a stated 8.09B. It was dropped when the examples were rebuilt around the legend block. The anti-pattern list, which still forbade outright what the honest-data rule permits once disclosed, now agrees with it. Also nudges the info mark onto the label row of its neighbours rather than the vertical centre of its own cell, so the bottom row reads as one row. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(treemap): reject ambiguous share metadata --------- Co-authored-by: 0xDarkMatter <0xDarkMatter@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Cathryn Lavery
co-authored by
0xDarkMatter
Claude Fable 5
0xDarkMatter
Claude Fable 5
parent
91fb4f243b
commit
e011b04af8
@@ -92,14 +92,28 @@ def seed_package(
|
||||
],
|
||||
},
|
||||
)
|
||||
skill = root / "skills" / PLUGIN_NAME / "SKILL.md"
|
||||
skill.parent.mkdir(parents=True, exist_ok=True)
|
||||
skill.write_text(f"---\nname: {PLUGIN_NAME}\n---\n", encoding="utf-8")
|
||||
write_skill(root, version)
|
||||
command = root / "commands" / "export-diagram.md"
|
||||
command.parent.mkdir(parents=True, exist_ok=True)
|
||||
command.write_text("Export the diagram.\n", encoding="utf-8")
|
||||
|
||||
|
||||
def write_skill(root: Path, version: str) -> None:
|
||||
"""Fixture SKILL.md, whose metadata.version tracks the manifest MAJOR.MINOR.
|
||||
|
||||
The verifier gates that relationship, so a fixture that omits it is not a
|
||||
valid package and would only prove the gate can be tripped by its own
|
||||
test data.
|
||||
"""
|
||||
skill = root / "skills" / PLUGIN_NAME / "SKILL.md"
|
||||
skill.parent.mkdir(parents=True, exist_ok=True)
|
||||
minor = ".".join(version.split(".")[:2])
|
||||
skill.write_text(
|
||||
f'---\nname: {PLUGIN_NAME}\nmetadata:\n version: "{minor}"\n---\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
|
||||
@contextmanager
|
||||
def package_repo(include_factory: bool = True) -> Iterator[Path]:
|
||||
with tempfile.TemporaryDirectory() as scratch:
|
||||
@@ -130,6 +144,10 @@ def set_versions(
|
||||
payload = json.loads((root / relative).read_text(encoding="utf-8"))
|
||||
payload["version"] = version
|
||||
write_json(root / relative, payload)
|
||||
# Keep the fixture's SKILL.md in step with the Claude manifest, so these
|
||||
# cases exercise the version rules they are about rather than tripping the
|
||||
# metadata-drift gate every time.
|
||||
write_skill(root, claude)
|
||||
|
||||
|
||||
def set_existing_versions(root: Path, version: str) -> None:
|
||||
@@ -261,6 +279,32 @@ def test_verifier() -> None:
|
||||
"could not read",
|
||||
)
|
||||
|
||||
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}\n---\n\nExample only:\nversion: "1.2"\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
expect_failure(
|
||||
"version line outside 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}\nversion: "1.2"\nmetadata:\n owner: test\n---\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
expect_failure(
|
||||
"version outside metadata mapping",
|
||||
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"))
|
||||
|
||||
@@ -263,6 +263,139 @@ def main() -> int:
|
||||
else:
|
||||
print("OK: an unlabelled sliver does not disable the area check")
|
||||
|
||||
# 6. The unlabelled sliver, both directions. This is the gap issue #89
|
||||
# reported: with the basis derived from in-cell labels, the one cell
|
||||
# carrying no label was the one cell nothing verified — and it is the
|
||||
# cell a 4px grid distorts most. Both rects of the pair move, because
|
||||
# a mask and a body at different sizes are two cells, not one.
|
||||
for label, width, direction in (("oversized", "24", "larger"), ("undersized", "8", "smaller")):
|
||||
mutated = source.replace(
|
||||
'<rect x="940" y="296" width="16" height="124"',
|
||||
f'<rect x="940" y="296" width="{width}" height="124"',
|
||||
)
|
||||
if mutated == source:
|
||||
failures.append(f"could not build the {label}-sliver fixture (anchor moved)")
|
||||
continue
|
||||
code, output = run(write(directory, f"sliver-{label}.html", mutated))
|
||||
if code == 0:
|
||||
failures.append(
|
||||
f"{label} unlabelled sliver was accepted — the cell with no label "
|
||||
f"is exactly the one that must still be checked"
|
||||
)
|
||||
elif "declares" not in output:
|
||||
failures.append(f"{label} sliver reported without an area finding: {output.strip()}")
|
||||
else:
|
||||
print(f"OK: an unlabelled sliver drawn {direction} than it declares is rejected")
|
||||
|
||||
# 7. Fail closed. A cell with no declared share is unverifiable, and
|
||||
# reporting OK on it is the defect, not a tolerable gap.
|
||||
stripped = source.replace(' data-share="0.56"', "", 1)
|
||||
if stripped == source:
|
||||
failures.append("could not build the missing-metadata fixture (anchor moved)")
|
||||
else:
|
||||
code, output = run(write(directory, "nometa.html", stripped))
|
||||
if code == 0:
|
||||
failures.append("a cell with no data-share was accepted — must fail closed")
|
||||
elif "no data-share" not in output:
|
||||
failures.append(f"missing metadata reported without the right finding: {output.strip()}")
|
||||
else:
|
||||
print("OK: a cell with no declared share fails closed")
|
||||
|
||||
# 8. A label that disagrees with the metadata it sits on.
|
||||
divergent = source.replace("4.78B · 59% of world", "4.78B · 42% of world")
|
||||
if divergent == source:
|
||||
failures.append("could not build the label-divergence fixture (anchor moved)")
|
||||
else:
|
||||
code, output = run(write(directory, "divergent.html", divergent))
|
||||
if code == 0:
|
||||
failures.append("a label contradicting its own data-share was accepted")
|
||||
elif "same fact" not in output:
|
||||
failures.append(f"divergence reported without the right finding: {output.strip()}")
|
||||
else:
|
||||
print("OK: a label contradicting its cell's declared share is rejected")
|
||||
|
||||
# 9. Invalid shares fail closed without raising or slipping through
|
||||
# float edge cases. Zero used to divide by zero; NaN/Infinity made
|
||||
# comparisons false; negative and >100 values are not shares of a
|
||||
# whole even though float() accepts them.
|
||||
for label, value in (
|
||||
("non-numeric", "unknown"),
|
||||
("zero", "0"),
|
||||
("negative", "-1"),
|
||||
("non-finite-nan", "NaN"),
|
||||
("non-finite-infinity", "Infinity"),
|
||||
("over-100", "101"),
|
||||
):
|
||||
invalid = source.replace('data-share="59.04"', f'data-share="{value}"', 1)
|
||||
if invalid == source:
|
||||
failures.append(f"could not build the {label}-share fixture")
|
||||
continue
|
||||
code, output = run(write(directory, f"share-{label}.html", invalid))
|
||||
if code == 0:
|
||||
failures.append(f"{label} data-share was accepted")
|
||||
elif "invalid share metadata" not in output:
|
||||
failures.append(f"{label} share lacked a metadata finding: {output.strip()}")
|
||||
else:
|
||||
print(f"OK: {label} data-share fails closed")
|
||||
|
||||
# 10. The mask/body pair is one logical cell. If both copies state a
|
||||
# share, they must agree; retaining whichever rect was parsed first
|
||||
# silently discards contradictory source data.
|
||||
asia_mask = '<rect x="40" y="40" width="532" height="380" rx="2" fill="#f5f5f5"/>'
|
||||
conflicting = source.replace(
|
||||
asia_mask,
|
||||
asia_mask.replace(' fill=', ' data-share="42" fill='),
|
||||
1,
|
||||
)
|
||||
if conflicting == source:
|
||||
failures.append("could not build the conflicting-twin fixture")
|
||||
else:
|
||||
code, output = run(write(directory, "conflicting-twins.html", conflicting))
|
||||
if code == 0:
|
||||
failures.append("conflicting duplicate data-share values were accepted")
|
||||
elif "conflicting data-share" not in output:
|
||||
failures.append(f"conflicting twins lacked the right finding: {output.strip()}")
|
||||
else:
|
||||
print("OK: conflicting mask/body shares are rejected")
|
||||
|
||||
asia_body = (
|
||||
'<rect x="40" y="40" width="532" height="380" rx="2" '
|
||||
'data-share="59.04" fill="rgba(235,108,54,0.16)" '
|
||||
'stroke="#eb6c36" stroke-width="1.5"/>'
|
||||
)
|
||||
triplicated = source.replace(asia_body, f"{asia_body}\n {asia_body}", 1)
|
||||
if triplicated == source:
|
||||
failures.append("could not build the triplicated-cell fixture")
|
||||
else:
|
||||
code, output = run(write(directory, "triplicated-cell.html", triplicated))
|
||||
if code == 0:
|
||||
failures.append("a triplicated logical cell was silently deduplicated")
|
||||
elif "duplicate rects" not in output:
|
||||
failures.append(f"triplicated cell lacked the right finding: {output.strip()}")
|
||||
else:
|
||||
print("OK: extra duplicate cell rects are rejected")
|
||||
|
||||
# 11. The shares describe one whole, so a plausible individual number
|
||||
# cannot hide an incomplete or over-counted set.
|
||||
incomplete_total = source.replace('data-share="59.04"', 'data-share="50"', 1)
|
||||
code, output = run(write(directory, "incomplete-share-total.html", incomplete_total))
|
||||
if code == 0:
|
||||
failures.append("data-share values that do not total the whole were accepted")
|
||||
elif "not approximately 100%" not in output:
|
||||
failures.append(f"bad share total lacked the right finding: {output.strip()}")
|
||||
else:
|
||||
print("OK: shares must total approximately 100%")
|
||||
|
||||
# 12. A treemap with too few parseable cells is unchecked, not clean.
|
||||
unparsable = source.replace(' rx="2"', ' rx="3"')
|
||||
code, output = run(write(directory, "too-few-cells.html", unparsable))
|
||||
if code == 0:
|
||||
failures.append("a treemap with no parseable cells was accepted")
|
||||
elif "expected at least 3 parseable treemap cells" not in output:
|
||||
failures.append(f"too-few-cells fixture lacked the right finding: {output.strip()}")
|
||||
else:
|
||||
print("OK: too few parseable cells fails closed")
|
||||
|
||||
for failure in failures:
|
||||
print(f"FAIL: {failure}")
|
||||
if failures:
|
||||
|
||||
@@ -280,6 +280,83 @@ 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 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.
|
||||
"""
|
||||
lines = text.splitlines()
|
||||
if not lines or lines[0].strip() != "---":
|
||||
return None
|
||||
try:
|
||||
closing = next(
|
||||
index for index in range(1, len(lines)) if lines[index].strip() == "---"
|
||||
)
|
||||
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
|
||||
|
||||
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:
|
||||
continue
|
||||
match = re.fullmatch(
|
||||
r"\s*version:\s*(?:\"([^\"]+)\"|'([^']+)'|([0-9]+(?:\.[0-9]+)*))\s*(?:#.*)?",
|
||||
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
|
||||
|
||||
|
||||
def verify_skill_metadata_version(root: Path, manifests: dict[str, dict[str, Any]], errors: list[str]) -> None:
|
||||
"""SKILL.md's metadata version must track the manifests' MAJOR.MINOR.
|
||||
|
||||
The manifests are bumped by a script; SKILL.md's `metadata.version` is
|
||||
hand-maintained, so a minor release drifts silently - 2.4 sat against
|
||||
2.5.0 manifests until a post-merge sweep caught it. Nothing else reads
|
||||
both numbers, so nothing else can notice.
|
||||
"""
|
||||
skill = root / "skills" / PLUGIN_NAME / "SKILL.md"
|
||||
if not skill.is_file():
|
||||
errors.append(f"missing {skill.relative_to(root).as_posix()}")
|
||||
return
|
||||
declared = parse_frontmatter_metadata_version(skill.read_text(encoding="utf-8"))
|
||||
if declared is None:
|
||||
errors.append("SKILL.md frontmatter has no metadata.version")
|
||||
return
|
||||
manifest_version = str(manifests["Claude"].get("version", ""))
|
||||
expected = ".".join(manifest_version.split(".")[:2])
|
||||
if declared != expected:
|
||||
errors.append(
|
||||
f"SKILL.md metadata.version {declared!r} must track the manifest "
|
||||
f"MAJOR.MINOR {expected!r} (manifests are at {manifest_version})"
|
||||
)
|
||||
|
||||
|
||||
def verify_package(root: Path, base_ref: str) -> list[str]:
|
||||
errors: list[str] = []
|
||||
manifests: dict[str, dict[str, Any]] = {}
|
||||
@@ -291,6 +368,7 @@ def verify_package(root: Path, base_ref: str) -> list[str]:
|
||||
if len(manifests) == len(MANIFEST_PATHS):
|
||||
verify_versions(root, base_ref, manifests, errors)
|
||||
verify_manifest_identity(manifests, errors)
|
||||
verify_skill_metadata_version(root, manifests, errors)
|
||||
verify_codex_skill_path(root, manifests["Codex"], errors)
|
||||
verify_marketplaces(root, errors)
|
||||
return errors
|
||||
|
||||
+140
-48
@@ -38,6 +38,7 @@ from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import html
|
||||
import math
|
||||
import re
|
||||
import sys
|
||||
import unicodedata
|
||||
@@ -46,13 +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[^>]*?"
|
||||
r'\bx="(?P<x>-?[\d.]+)"\s+y="(?P<y>-?[\d.]+)"\s+'
|
||||
r'width="(?P<w>[\d.]+)"\s+height="(?P<h>[\d.]+)"[^>]*?'
|
||||
r'rx="2"',
|
||||
re.IGNORECASE,
|
||||
)
|
||||
CELL_RE = re.compile(r'<rect\b(?P<attrs>[^>]*\brx="2"[^>]*?)/?>', 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>[^"]*)"')
|
||||
@@ -87,10 +82,26 @@ MARKER_EPSILON = 0.01
|
||||
|
||||
|
||||
class Box:
|
||||
__slots__ = ("x", "y", "w", "h", "offset")
|
||||
__slots__ = (
|
||||
"x",
|
||||
"y",
|
||||
"w",
|
||||
"h",
|
||||
"offset",
|
||||
"share",
|
||||
"share_errors",
|
||||
"rect_count",
|
||||
)
|
||||
|
||||
def __init__(self, x: float, y: float, w: float, h: float, offset: int = 0) -> None:
|
||||
def __init__(
|
||||
self, x: float, y: float, w: float, h: float, offset: int = 0, share: float | None = None
|
||||
) -> None:
|
||||
self.x, self.y, self.w, self.h, self.offset = x, y, w, h, offset
|
||||
# Declared share of the whole, from data-share. None means the cell
|
||||
# never stated one, which for a treemap is itself a finding.
|
||||
self.share = share
|
||||
self.share_errors: list[str] = []
|
||||
self.rect_count = 1
|
||||
|
||||
@property
|
||||
def right(self) -> float:
|
||||
@@ -137,25 +148,64 @@ def estimated_advance(text: str, mono: bool) -> float:
|
||||
|
||||
|
||||
def parse_cells(source: str) -> list[Box]:
|
||||
"""Deduped cell rects. Each cell is painted twice: paper mask, then body."""
|
||||
"""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
|
||||
merged across duplicates rather than read off whichever came first.
|
||||
"""
|
||||
seen: list[Box] = []
|
||||
for m in CELL_RE.finditer(source):
|
||||
box = Box(
|
||||
float(m.group("x")),
|
||||
float(m.group("y")),
|
||||
float(m.group("w")),
|
||||
float(m.group("h")),
|
||||
m.start(),
|
||||
)
|
||||
attrs = {a.group("name"): a.group("value") for a in ATTR_RE.finditer(m.group("attrs"))}
|
||||
try:
|
||||
box = Box(
|
||||
float(attrs["x"]),
|
||||
float(attrs["y"]),
|
||||
float(attrs["width"]),
|
||||
float(attrs["height"]),
|
||||
m.start(),
|
||||
)
|
||||
except (KeyError, ValueError):
|
||||
continue
|
||||
if "data-share" in attrs:
|
||||
raw_share = attrs["data-share"]
|
||||
try:
|
||||
parsed_share = float(raw_share)
|
||||
except ValueError:
|
||||
box.share_errors.append(f"data-share {raw_share!r} is not numeric")
|
||||
else:
|
||||
if not math.isfinite(parsed_share):
|
||||
box.share_errors.append(f"data-share {raw_share!r} is not finite")
|
||||
elif not 0 < parsed_share <= 100:
|
||||
box.share_errors.append(
|
||||
f"data-share {raw_share!r} must be greater than 0 and at most 100"
|
||||
)
|
||||
else:
|
||||
box.share = parsed_share
|
||||
if box.area < CELL_MIN_AREA or box.w > CELL_MAX_W or box.h > CELL_MAX_H:
|
||||
continue
|
||||
if any(
|
||||
abs(s.x - box.x) < 0.01
|
||||
and abs(s.y - box.y) < 0.01
|
||||
and abs(s.w - box.w) < 0.01
|
||||
and abs(s.h - box.h) < 0.01
|
||||
for s in seen
|
||||
):
|
||||
twin = next(
|
||||
(
|
||||
s
|
||||
for s in seen
|
||||
if abs(s.x - box.x) < 0.01
|
||||
and abs(s.y - box.y) < 0.01
|
||||
and abs(s.w - box.w) < 0.01
|
||||
and abs(s.h - box.h) < 0.01
|
||||
),
|
||||
None,
|
||||
)
|
||||
if twin is not None:
|
||||
twin.rect_count += 1
|
||||
twin.share_errors.extend(box.share_errors)
|
||||
if twin.share is not None and box.share is not None and not math.isclose(
|
||||
twin.share, box.share, rel_tol=0.0, abs_tol=1e-9
|
||||
):
|
||||
twin.share_errors.append(
|
||||
f"duplicate rects declare conflicting data-share values "
|
||||
f"{twin.share:g} and {box.share:g}"
|
||||
)
|
||||
if twin.share is None:
|
||||
twin.share = box.share
|
||||
continue
|
||||
seen.append(box)
|
||||
return seen
|
||||
@@ -223,7 +273,10 @@ def check(path: Path) -> list[str]:
|
||||
source = path.read_text(encoding="utf-8")
|
||||
cells = parse_cells(source)
|
||||
if len(cells) < 3:
|
||||
return []
|
||||
return [
|
||||
f"{path.name}: expected at least 3 parseable treemap cells, found {len(cells)} — "
|
||||
"the area contract cannot be verified"
|
||||
]
|
||||
|
||||
findings: list[str] = []
|
||||
labels: dict[int, list[str]] = {index: [] for index in range(len(cells))}
|
||||
@@ -306,31 +359,70 @@ def check(path: Path) -> list[str]:
|
||||
"drop the marker and identify the sliver in the legend"
|
||||
)
|
||||
|
||||
claims = {index: parse_claim(" ".join(texts)) for index, texts in labels.items()}
|
||||
# Only compare on a basis every cell states, so the shares share a
|
||||
# denominator. Percentages win when present: they are the claim itself.
|
||||
percentages = {i: p for i, (p, _) in claims.items() if p}
|
||||
magnitudes = {i: v for i, (_, v) in claims.items() if v}
|
||||
known = percentages if len(percentages) >= len(magnitudes) else magnitudes
|
||||
# Every cell is checked against its OWN declared share, so the cell most
|
||||
# likely to be wrong is not the one exempt from checking. Deriving the
|
||||
# basis from in-cell labels instead leaves an unlabelled sliver - exactly
|
||||
# the cell a 4px grid distorts most - verified by nothing at all.
|
||||
for cell in cells:
|
||||
for error in cell.share_errors:
|
||||
findings.append(
|
||||
f"{path.name}:{line_of(source, cell.offset)}: cell {cell.w:g}x{cell.h:g} "
|
||||
f"has invalid share metadata: {error}"
|
||||
)
|
||||
if cell.rect_count > 2:
|
||||
findings.append(
|
||||
f"{path.name}:{line_of(source, cell.offset)}: cell {cell.w:g}x{cell.h:g} "
|
||||
f"is painted by {cell.rect_count} duplicate rects — expected at most a mask/body pair"
|
||||
)
|
||||
|
||||
# Normalise both sides over the labelled cells only. A sliver too small to
|
||||
# label is legal - it keeps its honest area and is named in the source line
|
||||
# - and it must not switch this check off for the cells that do carry a
|
||||
# claim, which is what comparing against the full drawn area would do.
|
||||
if len(known) >= 3:
|
||||
drawn_total = sum(cells[index].area for index in known)
|
||||
value_total = sum(known.values())
|
||||
for index, value in sorted(known.items()):
|
||||
cell = cells[index]
|
||||
area_share = cell.area / drawn_total * 100
|
||||
value_share = value / value_total * 100
|
||||
relative = (area_share - value_share) / value_share * 100
|
||||
if abs(relative) > AREA_TOLERANCE:
|
||||
undeclared = [cell for cell in cells if cell.share is None and not cell.share_errors]
|
||||
if undeclared:
|
||||
# Fail closed. A treemap whose cells cannot be parsed is not a passing
|
||||
# treemap; it is an unchecked one, and saying "OK" to it is the bug.
|
||||
for cell in undeclared:
|
||||
findings.append(
|
||||
f"{path.name}:{line_of(source, cell.offset)}: cell {cell.w:g}x{cell.h:g} "
|
||||
f"has no data-share, so its area is unverifiable — every treemap cell "
|
||||
f"must declare its share of the whole, including cells too small to label"
|
||||
)
|
||||
return findings
|
||||
|
||||
drawn_total = sum(cell.area for cell in cells)
|
||||
if any(cell.share_errors for cell in cells):
|
||||
return findings
|
||||
|
||||
share_total = sum(cell.share for cell in cells if cell.share is not None)
|
||||
if not 99.0 <= share_total <= 101.0:
|
||||
findings.append(
|
||||
f"{path.name}: data-share values total {share_total:g}%, not approximately 100% — "
|
||||
"every cell must declare its share of the same whole"
|
||||
)
|
||||
return findings
|
||||
|
||||
for index, cell in enumerate(cells):
|
||||
assert cell.share is not None
|
||||
area_share = cell.area / drawn_total * 100
|
||||
declared = cell.share / share_total * 100
|
||||
relative = (area_share - declared) / declared * 100
|
||||
if abs(relative) > AREA_TOLERANCE:
|
||||
findings.append(
|
||||
f"{path.name}:{line_of(source, cell.offset)}: cell {cell.w:g}x{cell.h:g} "
|
||||
f"draws {area_share:.2f}% of the area but declares {declared:.2f}% — "
|
||||
f"{relative:+.1f}% relative. Area is the only encoding; resize the cell"
|
||||
)
|
||||
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:
|
||||
drift = abs(percentage - declared)
|
||||
# 1pp covers honest rounding of a displayed integer percentage.
|
||||
if drift > 1.0:
|
||||
findings.append(
|
||||
f"{path.name}:{line_of(source, cell.offset)}: cell "
|
||||
f"{cell.w:g}x{cell.h:g} labelled {' '.join(labels[index])[:30]!r} draws "
|
||||
f"{area_share:.2f}% of the area for a {value_share:.2f}% value — "
|
||||
f"{relative:+.1f}% relative. Area is the only encoding; resize the cell"
|
||||
f"{path.name}:{line_of(source, cell.offset)}: cell {cell.w:g}x{cell.h:g} "
|
||||
f"is labelled {percentage:g}% but declares {declared:.2f}% — "
|
||||
f"the label and data-share must state the same fact"
|
||||
)
|
||||
return findings
|
||||
|
||||
|
||||
Reference in New Issue
Block a user