From 6cb482adc121fdb17197750d75894a63db3cd9f6 Mon Sep 17 00:00:00 2001 From: Kieran Batchelli-Hennessey Date: Tue, 3 Nov 2026 14:11:55 -0800 Subject: [PATCH] Reject malformed benchmark posts instead of crashing the review Several fields in a post could raise TypeError, ValueError, OverflowError or AttributeError instead of Rejected: timestamps without a time zone or that don't parse, integers too large for a float, a list as the model id or workload, and the optional load_seconds, peak_memory_bytes, settings and check fields that build_dataset uses. Because parsed() runs over every post in the category, one such post broke every later review and the leaderboard build. --- Scripts/benchmark_discussions.py | 27 ++++++++++++++++++++++----- Scripts/test_benchmark_discussions.py | 9 +++++++++ 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/Scripts/benchmark_discussions.py b/Scripts/benchmark_discussions.py index bfeae978..a467d4e9 100644 --- a/Scripts/benchmark_discussions.py +++ b/Scripts/benchmark_discussions.py @@ -138,7 +138,7 @@ def number(value, name: str, minimum: float = 0, maximum: float = math.inf, raise Rejected(f"`{name}` is not a number.") if integer and not isinstance(value, int): raise Rejected(f"`{name}` is not a whole number.") - if not math.isfinite(value) or not minimum <= value <= maximum: + if (isinstance(value, float) and not math.isfinite(value)) or not minimum <= value <= maximum: raise Rejected(f"`{name}` is out of range.") return value @@ -151,6 +151,16 @@ def text(value, name: str, pattern: re.Pattern | None = None, limit: int = 120) return value +def timestamp(value, name: str) -> datetime: + try: + moment = parse_time(text(value, name)) + except ValueError: + raise Rejected(f"`{name}` is not a timestamp.") from None + if moment.tzinfo is None: + raise Rejected(f"`{name}` has no time zone.") + return moment + + def version_tuple(value: str) -> tuple[int, int, int]: match = VERSION.match(value) if not match: @@ -220,8 +230,8 @@ def validate_structure(result: dict) -> list[str]: if machine.get(key) is not None: number(machine[key], f"machine.{key}", 0, 256, integer=True) - started = parse_time(text(result.get("started_at"), "started_at")) - finished = parse_time(text(result.get("finished_at"), "finished_at")) + started = timestamp(result.get("started_at"), "started_at") + finished = timestamp(result.get("finished_at"), "finished_at") if finished < started: raise Rejected("The run finishes before it starts.") if started > datetime.now(timezone.utc) + timedelta(days=1): @@ -236,7 +246,7 @@ def validate_structure(result: dict) -> list[str]: for run in runs: if not isinstance(run, dict) or not isinstance(run.get("model"), dict): raise Rejected("A run is malformed.") - model_id = run["model"].get("id") + model_id = text(run["model"].get("id"), "model.id") if model_id not in CONFIG["models"]: raise Rejected(f"Unknown model `{model_id}`.") if model_id in seen: @@ -244,6 +254,11 @@ def validate_structure(result: dict) -> list[str]: seen.add(model_id) if run.get("status") not in ("completed", "failed", "skipped", "cancelled"): raise Rejected("A run has an unknown status.") + for key in ("settings", "check"): + if run.get(key) is not None and not isinstance(run[key], dict): + raise Rejected(f"`{key}` is malformed.") + if run.get("load_seconds") is not None: + number(run["load_seconds"], "load_seconds", 0, 36_000) trials = run.get("trials") if not isinstance(trials, list) or len(trials) > MAX_TRIALS: raise Rejected("A run's trials are malformed.") @@ -287,7 +302,7 @@ def validate_trial(trial, model_id: str) -> float: if not isinstance(trial, dict): raise Rejected("A trial is malformed.") workload = trial.get("workload") - if workload not in WORKLOAD_MAX_NEW: + if not isinstance(workload, str) or workload not in WORKLOAD_MAX_NEW: raise Rejected("A trial has an unknown workload.") number(trial.get("trial"), "trial", 1, MAX_TRIALS, integer=True) prompt = number(trial.get("prompt_tokens"), "prompt_tokens", 1, 70_000, integer=True) @@ -296,6 +311,8 @@ def validate_trial(trial, model_id: str) -> float: WORKLOAD_MAX_NEW[workload], integer=True) prefill = number(trial.get("prefill_seconds"), "prefill_seconds", 0, 36_000) decode = number(trial.get("decode_seconds"), "decode_seconds", 0, 36_000) + if trial.get("peak_memory_bytes") is not None: + number(trial["peak_memory_bytes"], "peak_memory_bytes", 0, 1 << 40, integer=True) if cached > prompt: raise Rejected("A trial reused more tokens than its prompt had.") if workload != "follow-up" and cached > 64: diff --git a/Scripts/test_benchmark_discussions.py b/Scripts/test_benchmark_discussions.py index 50656d7f..f24ea24c 100644 --- a/Scripts/test_benchmark_discussions.py +++ b/Scripts/test_benchmark_discussions.py @@ -146,6 +146,15 @@ def mutate(change): "duplicate model": lambda d: d["runs"].append(copy.deepcopy(d["runs"][0])), "nothing completed": lambda d: d["runs"][0].update(status="failed"), "string number": lambda d: d["runs"][0]["trials"][1].update(prompt_tokens="45"), + "huge number": lambda d: d["runs"][0]["trials"][1].update(prompt_tokens=10 ** 400), + "no time zone": lambda d: d.update(started_at="2026-10-09T08:00:00"), + "not a time": lambda d: d.update(finished_at="soon"), + "list model id": lambda d: d["runs"][0]["model"].update(id=[]), + "list workload": lambda d: d["runs"][0]["trials"][1].update(workload=[]), + "string load time": lambda d: d["runs"][0].update(load_seconds="3"), + "string peak memory": lambda d: d["runs"][0]["trials"][1].update(peak_memory_bytes="1"), + "check not an object": lambda d: d["runs"][0].update(check=1), + "settings not an object": lambda d: d["runs"][0].update(settings="x"), } for name, change in cases.items(): with self.subTest(name):