Add versioned benchmark JSON schema validation (v1) - #10
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The schema/tests do not currently enforce the required protocol field per Issue #3 acceptance criteria, so the validation guarantees are incomplete.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a versioned (v1) JSON Schema and pytest-based validation to make checked-in benchmark result JSON files machine-checkable and enforce required metadata fields.
Changes:
- Introduces Draft 2020-12 JSON Schema for benchmark results (
schema_version: 1). - Adds tests that validate the checked-in Qwen3 microbenchmark result file against the schema and exercise required-field failures.
File summaries
| File | Description |
|---|---|
| schemas/benchmark-v1.schema.json | Adds the v1 JSON Schema definition for benchmark result files. |
| tests/test_benchmark_schema.py | Adds pytest validation of the checked-in benchmark JSON against the schema plus negative required-field tests. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| "required": [ | ||
| "schema_version", | ||
| "claim_scope", | ||
| "model", | ||
| "model_revision", | ||
| "device", | ||
| "runs", | ||
| "summary", | ||
| "kind", | ||
| "prompt_tokens", | ||
| "generated_tokens", | ||
| "warmup_tokens", | ||
| "trials", | ||
| "budget", | ||
| "buffer_size", | ||
| "seed" | ||
| ], |
| schema = load_json(SCHEMA_PATH) | ||
| data = load_json(QWEN_RESULT_PATH) | ||
| validator = Draft202012Validator(schema) | ||
| errors = sorted(validator.iter_errors(data), key=lambda e: list(e.path)) | ||
| assert errors == [], [e.message for e in errors] |
| REQUIRED_FIELDS = [ | ||
| "schema_version", | ||
| "claim_scope", | ||
| "model", | ||
| "model_revision", | ||
| "device", | ||
| "runs", | ||
| "summary", | ||
| "kind", | ||
| "prompt_tokens", | ||
| "generated_tokens", | ||
| "warmup_tokens", | ||
| "trials", | ||
| "budget", | ||
| "buffer_size", | ||
| "seed", | ||
| ] |
|
Hi DaBestCode, could you please approve workflows for this PR? Thank you! |
|
Addressed Copilot review feedback in latest commits:
Local checks pass:
Could a maintainer please approve workflows so remaining CI can run? Thanks! |
|
could a maintainer please approve workflows for this PR so CI can continue? Thank you! |
DaBestCode
left a comment
DaBestCode
left a comment
There was a problem hiding this comment.
Thanks for taking this on. The schema direction is useful, but this needs four focused fixes before merge:
- Please remove the newly added top-level
protocolfield from both the fixture and schema. The issue guidance explicitly defines the protocol through the existing required fields (kind, token counts, trials, budget, buffer, and seed) and asks not to change the producer format solely to add a key. - Add
jsonschemato thetestextra inpyproject.toml. In a clean project environment,pytest -qcurrently fails during collection withModuleNotFoundError: No module named 'jsonschema'. - Add the schema compatibility policy to
results/README.md: additive optional fields remain compatible within v1; removing/renaming fields or changing their semantic meaning requires a new schema version. - Run
ruff formatontests/test_benchmark_schema.py;ruff format --check .currently fails on the final assertion.
Verified locally: ruff check . and mypy src pass; formatting and pytest fail for the reasons above. Please rerun the complete CONTRIBUTING.md check list after updating.
Fixes #3
Summary
This PR introduces schema-based validation for benchmark result JSON files.
Changes
schemas/benchmark-v1.schema.jsontests/test_benchmark_schema.pySchema/versioning policy
Validation run
ruff check .ruff format --check .mypy srcpytest -qAll checks pass locally.