diff --git a/docs/reference/application-benchmark-protocol.md b/docs/reference/application-benchmark-protocol.md index ef7184d0..6b548905 100644 --- a/docs/reference/application-benchmark-protocol.md +++ b/docs/reference/application-benchmark-protocol.md @@ -292,8 +292,14 @@ the protocol or a summary, including a field the checker does not otherwise read be finite and within ±(2⁵³ − 1), the interoperable JSON range. Any other number, including the `NaN` and `Infinity` that Python's JSON parser accepts, is refused as `protocol_number_out_of_range` or `summary_number_out_of_range`, alongside any code -for the field that holds it. A mean is the correctly rounded sum divided by the -count, so a mean at an exclusive ceiling gets the same verdict on every Python version. +for the field that holds it. A summary still gets the number code when +`summary_revision_mismatch` or `summary_workload_unknown` stops the rest of its check. +An integer of any length gets this code on every Python version, never an +unreadable-file code. No object may repeat a key, because the parser keeps only the +last value and the scan would never see the others: a repeated key is refused as +`protocol_key_duplicate` or `summary_key_duplicate` before any other check. A mean is +the correctly rounded sum divided by the count, so a mean at an exclusive ceiling gets +the same verdict on every Python version. These are acceptance thresholds, not measured achievements. @@ -320,8 +326,8 @@ record. It exits 1 when any verdict fails or is inconclusive. It exits 2, withou judging, when the protocol or summary is invalid: for example a summary for another cache class or scale point, a candidate marked `not_applicable`, a `not_applicable` record that carries samples, a negative sample, a planned count that is not a positive -integer, a number outside ±(2⁵³ − 1), or a value of the wrong type. Diagnostics are closed codes and never repeat -input values. +integer, a number outside ±(2⁵³ − 1), a repeated object key, or a value of the wrong +type. Diagnostics are closed codes and never repeat input values. A summary has this shape. `samples` lists only admitted samples; `flagged` and `failed` count the planned samples left out, so `samples` plus both counts never exceeds diff --git a/scripts/check-application-benchmark-protocol.py b/scripts/check-application-benchmark-protocol.py index 43fc956f..58b11dd0 100644 --- a/scripts/check-application-benchmark-protocol.py +++ b/scripts/check-application-benchmark-protocol.py @@ -12,7 +12,8 @@ workload pins, rotated lane order) are exact; budgets and sample counts are floors at their frozen values, which a revision may tighten. Every number anywhere in the protocol or a summary must be finite and lie within -MAX_JSON_NUMBER, including fields the checker does not otherwise read. +MAX_JSON_NUMBER, including fields the checker does not otherwise read, and no object +may repeat a key, so no value is hidden from that scan. Diagnostics are closed codes from DIAGNOSTICS; they never echo input values or paths. A budget verdict is one of VERDICTS. A statistic is judged only from complete, sufficient, admitted samples: missing, short, flagged or unaccounted samples are @@ -152,6 +153,11 @@ # integer range. Bounding every number keeps each statistic and ratio finite and keeps # Python from converting an unbounded JSON integer to a float. MAX_JSON_NUMBER = 2 ** 53 - 1 +# Decimal digits in MAX_JSON_NUMBER. An integer literal with more digits is out of range +# whatever its value, so it is never converted exactly: Python 3.11 and later refuse to +# convert one of more than 4300 digits, which would turn this range error into an +# unreadable file on some interpreters and not others. +MAX_JSON_INTEGER_DIGITS = len(str(MAX_JSON_NUMBER)) DIAGNOSTICS = frozenset({ "protocol_unreadable", @@ -199,9 +205,11 @@ "observer_policy_missing", "known_limit_malformed", "protocol_number_out_of_range", + "protocol_key_duplicate", "summary_unreadable", "summary_not_object", "summary_number_out_of_range", + "summary_key_duplicate", "summary_revision_mismatch", "summary_workload_unknown", "summary_cache_class_mismatch", @@ -335,8 +343,15 @@ def _numbers_in_range(value): def _same(value, frozen): - """Exact JSON equality: unlike ==, 4.0 is not 4 and true is not 1.""" - return json.dumps(value, sort_keys=True) == json.dumps(frozen, sort_keys=True) + """Exact JSON equality: unlike ==, 4.0 is not 4 and true is not 1. + + Every frozen value serializes, so a value that cannot (an integer beyond Python's + conversion limit, or a non-JSON object) is simply different, never an exception. + """ + try: + return json.dumps(value, sort_keys=True) == json.dumps(frozen, sort_keys=True) + except (TypeError, ValueError, RecursionError): + return False def _count(value): @@ -662,16 +677,18 @@ def validate_summary(protocol, summary): """ if not isinstance(summary, dict): return ["summary_not_object"], None + # The range scan comes first, so an out-of-range number is reported even when the + # revision or workload stops the summary from being checked further. + found = set() + if not _numbers_in_range(summary): + found.add("summary_number_out_of_range") if not _count(summary.get("protocol_revision")) or summary["protocol_revision"] != protocol["revision"]: - return ["summary_revision_mismatch"], None + return sorted(found | {"summary_revision_mismatch"}), None workload_id = summary.get("workload") workloads = {w["id"]: w for w in protocol["workloads"]} workload = workloads.get(workload_id) if isinstance(workload_id, str) else None if workload is None: - return ["summary_workload_unknown"], None - found = set() - if not _numbers_in_range(summary): - found.add("summary_number_out_of_range") + return sorted(found | {"summary_workload_unknown"}), None # Budgets are frozen for the warm class only; other classes are reported separately. if summary.get("cache_class") != protocol["cache_classes"]["budget_class"]: found.add("summary_cache_class_mismatch") @@ -828,9 +845,35 @@ def accepted(records): return bool(records) and all(record["verdict"] in ACCEPTING_VERDICTS for record in records) -def _load(path, unreadable): +class _DuplicateKey(Exception): + """An object repeated a key; json.loads would keep only the last value.""" + + +def _unique_object(pairs): + obj = dict(pairs) + if len(obj) != len(pairs): + raise _DuplicateKey + return obj + + +def _bounded_int(literal): + """Parse a JSON integer literal; one too long to be in range parses as just out of range. + + The sign is kept, and the result is still an int, so every type check treats it as + it would treat the exact value, and the range scan refuses it on every interpreter. + """ + if len(literal.lstrip("-")) > MAX_JSON_INTEGER_DIGITS: + return -(MAX_JSON_NUMBER + 1) if literal.startswith("-") else MAX_JSON_NUMBER + 1 + return int(literal) + + +def _load(path, unreadable, duplicate): + """Parse a JSON file, or return a closed code: `duplicate` for a repeated object key.""" try: - return json.loads(Path(path).read_text()), None + text = Path(path).read_text() + return json.loads(text, object_pairs_hook=_unique_object, parse_int=_bounded_int), None + except _DuplicateKey: + return None, duplicate except (OSError, ValueError, UnicodeDecodeError, RecursionError): return None, unreadable @@ -843,7 +886,7 @@ def main(argv=None): mode.add_argument("--evaluate", metavar="SUMMARY") args = parser.parse_args(argv) - protocol, problem = _load(args.protocol, "protocol_unreadable") + protocol, problem = _load(args.protocol, "protocol_unreadable", "protocol_key_duplicate") diagnostics = [problem] if problem else validate_protocol(protocol) if diagnostics: print(json.dumps({"status": "invalid", "diagnostics": diagnostics})) @@ -853,7 +896,7 @@ def main(argv=None): "orders": protocol["schedule"]["orders"]}, indent=2)) return 0 if args.evaluate: - summary, problem = _load(args.evaluate, "summary_unreadable") + summary, problem = _load(args.evaluate, "summary_unreadable", "summary_key_duplicate") diagnostics, records = ([problem], []) if problem else evaluate(protocol, summary) if diagnostics: print(json.dumps({"status": "invalid", "diagnostics": diagnostics})) diff --git a/tests/python/test_application_benchmark_protocol.py b/tests/python/test_application_benchmark_protocol.py index aa72ac2a..bd4cd1d6 100644 --- a/tests/python/test_application_benchmark_protocol.py +++ b/tests/python/test_application_benchmark_protocol.py @@ -16,7 +16,8 @@ protocol_check = importlib.util.module_from_spec(SPEC) SPEC.loader.exec_module(protocol_check) -PROTOCOL = json.loads((ROOT / "scripts/application-benchmark-protocol.json").read_text()) +PROTOCOL_TEXT = (ROOT / "scripts/application-benchmark-protocol.json").read_text() +PROTOCOL = json.loads(PROTOCOL_TEXT) PROTOCOL_DOC = (ROOT / "docs/reference/application-benchmark-protocol.md").read_text() PERFORMANCE_DOC = (ROOT / "docs/performance.md").read_text() MARKER = "PRIVATE-MARKER-9f2c" @@ -99,6 +100,14 @@ def evaluate_file(summary): return run_cli(["--evaluate", str(path)]) +def run_cli_text(text, flag): + """Run the CLI on raw JSON text, as a protocol (`--protocol`) or a summary (`--evaluate`).""" + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "input.json" + path.write_text(text) + return run_cli([flag, str(path)]) + + def node_paths(node, prefix=(), list_limit=None): """Yield the path of every node below `node`; `list_limit` caps indices per list.""" if isinstance(node, dict): @@ -226,6 +235,20 @@ def test_documents_carry_every_frozen_artifact_and_budget(self): self.assertIn(value, PERFORMANCE_DOC) self.assertIn("application-benchmark-protocol.md", PERFORMANCE_DOC) + def test_document_states_the_number_bound_and_its_codes(self): + # The page writes the bound as ±(2ⁿ − 1); every such bound must be MAX_JSON_NUMBER. + superscripts = str.maketrans("⁰¹²³⁴⁵⁶⁷⁸⁹", "0123456789") + bounds = re.findall(r"±\(2([⁰¹²³⁴⁵⁶⁷⁸⁹]+) − 1\)", PROTOCOL_DOC) + self.assertTrue(bounds) + # A bound written any other way would escape the check above. + self.assertEqual(PROTOCOL_DOC.count("±("), len(bounds)) + for exponent in bounds: + self.assertEqual(2 ** int(exponent.translate(superscripts)) - 1, protocol_check.MAX_JSON_NUMBER) + for code in ("protocol_number_out_of_range", "summary_number_out_of_range", "protocol_key_duplicate", + "summary_key_duplicate", "summary_revision_mismatch", "summary_workload_unknown"): + self.assertIn(code, protocol_check.DIAGNOSTICS) + self.assertIn(f"`{code}`", PROTOCOL_DOC) + def _recast_as_compose(item): for field in ("version", "commit", "asset_sha256", "tag", "asset", "capacity"): @@ -702,6 +725,82 @@ def test_a_deeply_nested_unread_field_is_scanned_without_raising(self): self.assertEqual(protocol_check.validate_protocol(mutated(lambda d: d.update(notes=deep))), ["protocol_number_out_of_range"]) + def test_an_out_of_range_number_is_reported_with_an_early_refusal(self): + # The revision and workload codes stop the check early; the number code still appears. + cases = { + "summary_revision_mismatch": lambda s: s.update(protocol_revision=2), + "summary_workload_unknown": lambda s: s.update(workload=MARKER), + } + for code, change in cases.items(): + for value in (float("nan"), 2 ** 53, -HUGE): + with self.subTest(code=code, value=repr(value)): + summary = small_summary() + change(summary) + summary["notes"] = value + expected = sorted([code, "summary_number_out_of_range"]) + self.assertEqual(protocol_check.evaluate(PROTOCOL, summary), (expected, [])) + self.assertEqual(evaluate_file(summary), (2, {"status": "invalid", "diagnostics": expected})) + # The number can be the revision itself: both codes describe the same field. + self.assertEqual(self.refused({**small_summary(), "protocol_revision": 2 ** 53}), + ["summary_number_out_of_range", "summary_revision_mismatch"]) + + def test_a_repeated_key_is_refused_so_no_number_is_shadowed(self): + # json.loads keeps the last value of a repeated key, so `NaN` here was never scanned. + frozen = PROTOCOL_TEXT.rstrip() + self.assertTrue(frozen.endswith("}")) + shadowed = frozen[:-1] + ', "notes": NaN, "notes": 1}' + self.assertEqual(run_cli_text(shadowed, "--protocol"), + (2, {"status": "invalid", "diagnostics": ["protocol_key_duplicate"]})) + self.assertEqual(run_cli_text(frozen[:-1] + ', "notes": 1}', "--protocol")[0], 0) + nested = frozen.replace('"id": "candidate",', '"id": "candidate", "id": "candidate",', 1) + self.assertNotEqual(nested, frozen) + self.assertEqual(run_cli_text(nested, "--protocol"), + (2, {"status": "invalid", "diagnostics": ["protocol_key_duplicate"]})) + summary = json.dumps(small_summary()) + self.assertEqual(run_cli_text(summary, "--evaluate")[0], 0) + repeated = summary.replace('"cache_class": "warm"', '"cache_class": NaN, "cache_class": "warm"', 1) + self.assertNotEqual(repeated, summary) + self.assertEqual(run_cli_text(repeated, "--evaluate"), + (2, {"status": "invalid", "diagnostics": ["summary_key_duplicate"]})) + + def test_integer_literals_of_any_length_get_the_range_code_on_every_interpreter(self): + # Python 3.11+ refuses to convert an integer of more than 4300 digits, which made a + # 5000-digit literal `protocol_unreadable` there and out of range on 3.9. + limit = protocol_check.MAX_JSON_NUMBER + frozen = PROTOCOL_TEXT.rstrip()[:-1] + for literal in ("9" * 5000, "-" + "9" * 5000, str(limit + 1), str(10 ** 16), str(-(10 ** 16))): + with self.subTest(digits=len(literal)): + self.assertEqual(run_cli_text(frozen + ', "notes": ' + literal + "}", "--protocol"), + (2, {"status": "invalid", "diagnostics": ["protocol_number_out_of_range"]})) + summary = json.dumps({**small_summary(), "notes": 0}).replace('"notes": 0', '"notes": ' + literal) + self.assertEqual(run_cli_text(summary, "--evaluate"), + (2, {"status": "invalid", "diagnostics": ["summary_number_out_of_range"]})) + for literal in (str(limit), str(-limit), "0", "-0"): + with self.subTest(literal=literal): + self.assertEqual(run_cli_text(frozen + ', "notes": ' + literal + "}", "--protocol")[0], 0) + # A long literal in a read field gets the same codes as an exact out-of-range value there. + floor = PROTOCOL_TEXT.replace('"p95_min_samples": 60', '"p95_min_samples": ' + "9" * 5000, 1) + self.assertNotEqual(floor, PROTOCOL_TEXT) + exact = protocol_check.validate_protocol(mutated(lambda d: d["samples"].update(p95_min_samples=10 ** 400))) + self.assertEqual(run_cli_text(floor, "--protocol"), (2, {"status": "invalid", "diagnostics": exact})) + self.assertEqual(protocol_check._bounded_int("9" * 5000), limit + 1) + self.assertEqual(protocol_check._bounded_int("-" + "9" * 5000), -limit - 1) + self.assertEqual(protocol_check._bounded_int(str(limit)), limit) + + def test_an_integer_beyond_the_conversion_limit_never_raises_in_process(self): + # Exact identity checks serialize values; on Python 3.11+ a 5000-digit integer + # cannot be serialized, which must mean "different", not an exception. + enormous = 10 ** 5000 + for path in (("host", "logical_cpus"), ("lanes", 0, "commit"), ("workloads", 0, "pin"), + ("samples", "p95_min_samples"), ("notes",)): + with self.subTest(path=path): + found = protocol_check.validate_protocol(replace_at(PROTOCOL, path, enormous)) + self.assertIn("protocol_number_out_of_range", found) + self.assertTrue(set(found) <= protocol_check.DIAGNOSTICS) + summary = small_summary() + summary["notes"] = enormous + self.assertEqual(self.refused(summary), ["summary_number_out_of_range"]) + def test_an_overflowing_ratio_fails_and_the_report_stays_strict_json(self): summary = small_summary() summary["lanes"]["compose"]["warm_startup_seconds"] = entry([5e-324] * 10)