fix: seven defects an adversarial audit found, and the gates that missed them
Release / test (full, windows-latest, 3.11) (push) Canceled after 0s
Release / test (pdf, ubuntu-latest, 3.11) (push) Canceled after 0s
Release / test (pdf, ubuntu-latest, 3.12) (push) Canceled after 0s
Release / build (push) Canceled after 0s
Release / publish-pypi (push) Canceled after 0s
Release / github-release (push) Canceled after 0s
Release / test (full, windows-latest, 3.11) (push) Canceled after 0s
Release / test (pdf, ubuntu-latest, 3.11) (push) Canceled after 0s
Release / test (pdf, ubuntu-latest, 3.12) (push) Canceled after 0s
Release / build (push) Canceled after 0s
Release / publish-pypi (push) Canceled after 0s
Release / github-release (push) Canceled after 0s
Every one of these was reported ok/clean to the caller while being wrong.
Silently wrong numbers
- selection_filter(min_area) read `entity.dxf.get("area", 0.0)` for SOLID,
TRACE and HATCH, none of which carry that attribute, so every hatch
measured 0.0. A 1600 mm^2 hatch was excluded by min_area=1.0 while
`filtered_by` asserted the filter had run, and COM (which reads ActiveX
.Area) returned a different selection for the same call. It now answers
from the measurement `entity_measure` uses, so the two cannot disagree.
- Every headless dimension rendered through ezdxf's bare `Standard` style:
1.0 mm text with a comma decimal marker, against ISO 3098's 2.5 mm minimum
and ISO 129's point -- while the document header itself said $DIMTXT 2.5.
`ezdxf.new(setup=True)` is not the fix; measured, it renders a 100 mm
dimension as "10000". The attributes are now stored explicitly.
- Saving writes `dimrnd = 0.0` onto the Standard style, and ezdxf applies
`xround(value, dimrnd)` whenever the attribute is present at all --
`xround(33.333, 0.0)` is 33. So on any reopened file, including one this
server saved, every dimension was rounded to a whole unit before DIMDEC
formatted it: a 12.75 mm feature dimensioned 13. AutoCAD means DIMRND 0 as
*no* rounding; the sentinel is now discarded at every load site, and a real
0.5 mm rounding is left alone.
- The DIM* header fold gave out-of-range values teeth they did not have when
they died in the header: dim_decimals=-1 returned ok and then killed the
next dimension with `ValueError: Format specifier missing precision` from
inside ezdxf, and 40 printed forty digits of float noise onto the sheet.
Ranges are now AutoCAD's own (DIMDEC 0-8, DIMZIN 0-15, lengths positive).
Security
- security.py recorded the policy as "sanitize_command and sanitize_lisp each
have exactly ONE caller ... the policy is scoped to the CHANNEL". The
premise was false. `_ensure_linetype_loaded` and `ComBackend.linetype_load`
build `_-LINETYPE _LOAD {name} {file}` and hand it to SendCommand, which
treats a newline and ';' as segment separators -- so a `linetype=` argument
carrying either injected further AutoCAD commands, and the 36-verb denylist
never saw that channel. Guarded with the DXF symbol-name rule (the
injection characters fall out of it rather than being enumerated) plus
validate_path for the file argument, which was also the one path-taking
tool reaching the filesystem unchecked. The policy comment is corrected.
Gates that could not see
- drawing_critique returned zero issues on a quarantined document: all seven
checks wrapped their backend call in a bare `except Exception`. CLAUDE.md
rule 7 makes that call the gate an agent is told to trust. Only the
quarantine now escapes; an ordinary broken check still degrades.
`_check_dimension_legibility` also read `_doc` directly from the event loop
thread, missing the gate and performing exactly the unsynchronised access
the quarantine exists to prevent. `construction_clear` answered
"deleted: 0" -- there was nothing to clear -- for the same reason.
- The iso128 focus checked layer lineweights only, so a sheet lettered at
1.0 mm critiqued clean. It now reads the effective dimension height and
separator, header-then-style, in the order the renderer resolves.
- check_doc_numbers.py derived five values it never checked, and those were
the ones that had drifted. The collected-test count (README said 1274, the
tree had 1369) and the A/B pass-rate and Fixed columns are now gated.
- benchmarks/README.md's v1.4.0 table was impossible on its face -- 21
passing plus 5 fixed is 26, not 24 -- and contradicted the report it
linked, because `ab-v1.4.0-vs-v1.5.0.json` was regenerated from a tree that
already carried the dimension fixes and so never measured the v1.5.0 tag.
Replaced with a real run: ab-v1.4.0-vs-v1.5.1.json, 21/26 -> 26/26.
Release gate
- release.yml ran one ubuntu/3.11 lane, so a Windows-only defect could reach
PyPI with no Windows job having executed. That is not hypothetical: v1.5.0
shipped `except pywintypes.com_error` raising NameError wherever pywin32 is
absent. It now runs the same three lanes as ci.yml, Windows included.
1368 passed, 1 xfailed. ruff clean. Tool inventory and all 16 gated document
numbers match the tree.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
6b9860e68e
commit
ca1c79fd01
@@ -0,0 +1,214 @@
|
||||
"""Hold the numbers in the documents equal to the numbers in the tree.
|
||||
|
||||
Every figure this checks has been wrong at least once, and the expensive one was
|
||||
wrong in three files at the same time: `benchmarks/README.md` and `CLAUDE.md`
|
||||
described the A/B suite as 24 checks at 87.5% while the suite held 26 and the
|
||||
published report said 80.8%. Nothing caught it because each file was edited on
|
||||
its own and nobody re-read the other two.
|
||||
|
||||
The rule is the one the release snapshot already follows: a number a human
|
||||
types is a number that rots, so derive it and fail when the prose disagrees.
|
||||
|
||||
python scripts/check_doc_numbers.py # report and exit non-zero on drift
|
||||
python scripts/check_doc_numbers.py --list # show every derived value
|
||||
|
||||
`tests/test_release_consistency.py` runs this as a gate.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
|
||||
|
||||
def _read(relative: str) -> str:
|
||||
return (ROOT / relative).read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _json(relative: str) -> dict:
|
||||
return json.loads(_read(relative))
|
||||
|
||||
|
||||
def _collected_tests() -> int:
|
||||
"""How many tests pytest actually collects.
|
||||
|
||||
On the same README line as tools/resources/prompts, and the only number on
|
||||
it that nothing derived -- so it drifted by 95 while its neighbours stayed
|
||||
exact. Collection only; running the suite here would make the gate cost a
|
||||
minute.
|
||||
"""
|
||||
env = dict(os.environ, AUTOCAD_MCP_BACKEND="ezdxf")
|
||||
result = subprocess.run(
|
||||
[sys.executable, "-m", "pytest", "--collect-only", "-q"],
|
||||
cwd=ROOT,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env=env,
|
||||
timeout=300,
|
||||
)
|
||||
match = re.search(r"(\d+) tests collected", result.stdout)
|
||||
if not match:
|
||||
raise SystemExit(
|
||||
"check_doc_numbers: could not read a collected-test count from pytest.\n"
|
||||
+ result.stdout[-2000:]
|
||||
)
|
||||
return int(match.group(1))
|
||||
|
||||
|
||||
def derived() -> dict[str, object]:
|
||||
"""Every number a document is allowed to quote, computed from the tree."""
|
||||
sys.path.insert(0, str(ROOT))
|
||||
inventory = _json("docs/tool-inventory.json")["totals"]
|
||||
ab = _json("benchmarks/results/published/ab-v1.5.0-vs-v1.5.1.json")["summary"]
|
||||
perf = {
|
||||
w["workload_id"]: w["wall_ms"]
|
||||
for w in _json("benchmarks/results/published/perf-ezdxf.json")["workloads"]
|
||||
}
|
||||
|
||||
from benchmarks import correctness_suite
|
||||
from benchmarks.tasks_v3 import TASKS_V3
|
||||
|
||||
return {
|
||||
"collected_tests": _collected_tests(),
|
||||
"tools": inventory["tools"],
|
||||
"resources": inventory["resources"],
|
||||
"prompts": inventory["prompts"],
|
||||
"groups": inventory["groups"],
|
||||
"ab_total": ab["total"],
|
||||
"ab_old_pass": ab["old_pass"],
|
||||
"ab_old_pct": ab["old_pct"],
|
||||
"ab_fixed": ab["fixed"],
|
||||
"ab_regressed": ab["regressed"],
|
||||
"suite_checks": len(correctness_suite.CHECKS),
|
||||
"v3_tasks": len(TASKS_V3),
|
||||
"perf_ms": perf,
|
||||
}
|
||||
|
||||
|
||||
#: (relative path, human label, regex over the file, callable -> expected value)
|
||||
#:
|
||||
#: Each pattern captures the number as group 1. A pattern that matches nothing is
|
||||
#: itself a failure: the sentence it was written for has moved, and a silent
|
||||
#: no-op gate is worse than no gate.
|
||||
CHECKS: list[tuple[str, str, str, str]] = [
|
||||
("README.md", "release snapshot tools", r"release snapshot:\*\*\s*(\d+) tools", "tools"),
|
||||
(
|
||||
"README.md",
|
||||
"release snapshot resources",
|
||||
r"release snapshot:.*?(\d+) resources",
|
||||
"resources",
|
||||
),
|
||||
(
|
||||
"README.md",
|
||||
"release snapshot prompts",
|
||||
r"release snapshot:.*?(\d+) prompt templates",
|
||||
"prompts",
|
||||
),
|
||||
("README.md", "A/B suite size", r"(\d+) deterministic headless checks", "ab_total"),
|
||||
("README.md", "A/B baseline pass", r"v1\.5\.0 \*\(baseline\)\* \| (\d+) / \d+", "ab_old_pass"),
|
||||
("README.md", "A/B suite denominator", r"v1\.5\.0 \*\(baseline\)\* \| \d+ / (\d+)", "ab_total"),
|
||||
("benchmarks/README.md", "A/B suite size", r"(\d+) checks, ezdxf backend", "ab_total"),
|
||||
(
|
||||
"benchmarks/README.md",
|
||||
"A/B baseline pass",
|
||||
r"\*\*v1\.5\.0\*\* \(baseline\)\s*\| (\d+) / \d+",
|
||||
"ab_old_pass",
|
||||
),
|
||||
(
|
||||
"benchmarks/README.md",
|
||||
"A/B suite denominator",
|
||||
r"\*\*v1\.5\.0\*\* \(baseline\)\s*\| \d+ / (\d+)",
|
||||
"ab_total",
|
||||
),
|
||||
(
|
||||
"README.md",
|
||||
"release snapshot collected tests",
|
||||
r"release snapshot:.*?(\d+) collected tests",
|
||||
"collected_tests",
|
||||
),
|
||||
(
|
||||
"README.md",
|
||||
"A/B baseline pass rate",
|
||||
r"v1\.5\.0 \*\(baseline\)\* \| \d+ / \d+ \| ([\d.]+) %",
|
||||
"ab_old_pct",
|
||||
),
|
||||
(
|
||||
"README.md",
|
||||
"A/B fixed count",
|
||||
r"v1\.5\.1\*\* \*\(this release\)\* \| \*\*\d+ / \d+\*\* \| \*\*\d+ %\*\* \| (\d+)",
|
||||
"ab_fixed",
|
||||
),
|
||||
(
|
||||
"benchmarks/README.md",
|
||||
"A/B baseline pass rate",
|
||||
r"\*\*v1\.5\.0\*\* \(baseline\)\s*\| \d+ / \d+ \| ([\d.]+) %",
|
||||
"ab_old_pct",
|
||||
),
|
||||
(
|
||||
"benchmarks/README.md",
|
||||
"A/B fixed count",
|
||||
r"\*\*v1\.5\.1\*\* \(this release\) \| \d+ / \d+ \| \d+ % \| (\d+)",
|
||||
"ab_fixed",
|
||||
),
|
||||
("CLAUDE.md", "A/B suite size", r"correctness_suite\.py` \((\d+) checks\)", "ab_total"),
|
||||
("CLAUDE.md", "v3 task count", r"the current set \((\d+) tasks", "v3_tasks"),
|
||||
]
|
||||
|
||||
|
||||
def run(verbose: bool = False) -> int:
|
||||
values = derived()
|
||||
|
||||
if values["suite_checks"] != values["ab_total"]:
|
||||
print(
|
||||
f"DRIFT correctness_suite.py has {values['suite_checks']} checks but the published "
|
||||
f"A/B report was run over {values['ab_total']} -- regenerate the report:\n"
|
||||
f" python benchmarks/compare_versions.py v1.5.0 "
|
||||
f"--json benchmarks/results/published/ab-v1.5.0-vs-v1.5.1.json"
|
||||
)
|
||||
return 1
|
||||
|
||||
if verbose:
|
||||
for key, value in values.items():
|
||||
print(f" {key:20s} {value}")
|
||||
print()
|
||||
|
||||
failures = 0
|
||||
for relative, label, pattern, key in CHECKS:
|
||||
text = _read(relative)
|
||||
match = re.search(pattern, text, re.DOTALL)
|
||||
expected = values[key]
|
||||
if match is None:
|
||||
print(
|
||||
f"DRIFT {relative}: the sentence carrying '{label}' no longer matches its pattern"
|
||||
)
|
||||
failures += 1
|
||||
continue
|
||||
found = type(expected)(match.group(1))
|
||||
if found != expected:
|
||||
print(f"DRIFT {relative}: {label} says {found}, the tree says {expected}")
|
||||
failures += 1
|
||||
elif verbose:
|
||||
print(f" ok {relative:24s} {label:24s} {found}")
|
||||
|
||||
if failures:
|
||||
print(f"\n{failures} document number(s) disagree with the tree.")
|
||||
return 1
|
||||
print(f"ok: {len(CHECKS)} document numbers match the tree")
|
||||
return 0
|
||||
|
||||
|
||||
def main() -> int:
|
||||
parser = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument("--list", action="store_true", help="Print every derived value")
|
||||
return run(verbose=parser.parse_args().list)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
raise SystemExit(main())
|
||||
Reference in New Issue
Block a user