chore: add eval-config validation check by adnanrhussain · Pull Request #120 · learning-commons-org/evaluators

adnanrhussain · 2026-06-25T23:47:46Z

Stacked on #119 (ahussain/nbstripout). Review/merge that first; this PR's diff is against that base.

What

Adds a second check to the harness: eval-configs, validating every evals/**/config.json against repo conventions.

python scripts/check.py eval-configs

Rules enforced

referenced files exist ($ref schemas, message source_path, fixtures path)
config + referenced schema/fixture files are valid JSON
each message's declared sha256 matches the prompt file (drift tripwire)
parser.kind == "structured_output"
message roles ∈ {system, user, assistant}
placeholder/template consistency (declared ⇄ used)

Check-only — config intent isn't safely auto-fixable (notably sha256, a deliberate drift tripwire).

It immediately caught a real leftover

intertextuality/config.json still declared a format_instructions placeholder (and stale parser sub-fields) from the old LangChain pattern — unused since the switch to structured_output. Fixed here so the repo is green.

Deferred (noted in `scripts/README.md`)

Fixture-label-in-enum validation needs a declared fixture→output-field mapping first, since some evaluators rename the field (complexity_score → complexity_level). Naming/structure conventions to follow.

codecov · 2026-06-29T19:22:37Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copilot

Pull request overview

Adds an eval-config check to the repo’s scripts/check.py harness to validate evals/**/config.json files against a shared JSON Schema and additional cross-file rules (refs, prompt files, sha drift, placeholders), and updates existing evaluators/fixtures/notebooks to conform to the new shared config + fixtures conventions.

Changes:

Introduces scripts/checks/eval_config.py and wires it into the harness; adds jsonschema dependency.
Adds shared schemas under evals/_schemas/ and updates evaluator config.json files to reference them via $schema.
Normalizes fixtures + notebooks to use fixtures.path and updates fixture expected keys (complexity_level → complexity_score) where applicable.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
scripts/requirements.txt	Adds `jsonschema` dependency for schema-based validation.
scripts/README.md	Documents the new `eval-config` check and updates planned follow-ups.
scripts/checks/eval_config.py	Implements the new evaluator config validation check (schema + cross-file rules).
scripts/checks/base.py	Adds shared helpers for locating evaluator configs and loading JSON.
scripts/checks/init.py	Registers `EvalConfig` in the harness check registry.
evals/standards/math-question-alignment/config.json	Migrates config to `$schema`-based shared schema and aligns step/preprocessing fields.
evals/purpose_evaluator.ipynb	Updates notebook fixture loading to `fixtures.path` and expected key naming.
evals/prompts/purpose/fixtures.json	Renames expected field to `complexity_score` to match output schema naming.
evals/prompts/purpose/config.json	Migrates to shared `$schema`, adds required evaluator fields, and standardizes fixtures path.
evals/literacy/qualitative-text-complexity/intertextuality/fixtures.json	Renames expected field to `complexity_score`.
evals/literacy/qualitative-text-complexity/intertextuality/example_notebook.ipynb	Updates notebook fixture loading + expected key naming to match shared conventions.
evals/literacy/qualitative-text-complexity/intertextuality/config.json	Removes obsolete LangChain-era fields and migrates to shared `$schema` conventions.
evals/feedback/productive-coaching-writing-feedback/manageable/example_notebook.ipynb	Updates fixture loading to `fixtures.path`.
evals/feedback/productive-coaching-writing-feedback/manageable/config.json	Migrates to shared `$schema`, adds required evaluator fields, standardizes fixtures path.
evals/feedback/productive-coaching-writing-feedback/appropriate-feedback/example_notebook.ipynb	Updates fixture loading to `fixtures.path`.
evals/feedback/productive-coaching-writing-feedback/appropriate-feedback/config.json	Migrates to shared `$schema`, adds required evaluator fields, standardizes fixtures path.
evals/feedback/productive-coaching-writing-feedback/anchored-in-student-response/example_notebook.ipynb	Updates fixture loading to `fixtures.path`.
evals/feedback/productive-coaching-writing-feedback/anchored-in-student-response/config.json	Migrates to shared `$schema`, adds required evaluator fields, standardizes fixtures path.
evals/feedback/productive-coaching-writing-feedback/actionable-revision/example_notebook.ipynb	Updates fixture loading to `fixtures.path`.
evals/feedback/productive-coaching-writing-feedback/actionable-revision/config.json	Migrates to shared `$schema`, adds required evaluator fields, standardizes fixtures path.
evals/feedback/productive-coaching-writing-feedback/acknowledges-strength/example_notebook.ipynb	Updates fixture loading to `fixtures.path`.
evals/feedback/productive-coaching-writing-feedback/acknowledges-strength/config.json	Migrates to shared `$schema`, adds required evaluator fields, standardizes fixtures path.
evals/_schemas/fixtures.schema.json	Adds shared fixtures schema contract (structural only).
evals/_schemas/config.schema.json	Adds shared config schema contract used by evaluator `config.json` via `$schema`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…fix notebook typo

czi-fsisenda

Looks good! Approved not to block, but got a couple of comments.
Separately, the configs are clean and well structured, but a lot less ergonomic for tinkering with the notebooks.
It's not impossible to do experimentation with the notebooks. Just more complex.
We should check how users of the notebooks feel about the updates.

czi-fsisenda · 2026-06-30T22:27:05Z

+    "evaluator": {
+      "type": "object",
+      "additionalProperties": false,
+      "required": ["id", "name", "description", "supported_grades"],


[P0] I don't think we should require supported_grades. We have seen evals that don't have grade as an input. input_schema is required and can enforce this.

czi-fsisenda · 2026-06-30T22:33:34Z

+        "id": { "type": "string", "minLength": 1 },
+        "name": { "type": "string", "minLength": 1 },
+        "description": { "type": "string", "minLength": 1 },


[P1?] "minLength": 1 feels like it implies a single character id, name, or description is ok. Can we go with "required": true or "allow_empty": false instead. Functionally the same, but no implicit endorsement of single character values for these fields.

czi-fsisenda · 2026-06-30T22:39:13Z

+          "type": "array",
+          "minItems": 1,
+          "items": {
+            "enum": ["K", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12"]


[P1] I've been using int for grades with 0 = 'K'. I don't see a strong reason to prefer one over the other. Maybe ints are safer for mapping grade bands to grades? Either way, we should be consistent.
[P0] And from earlier comment, I think supported_grades should be an input field not an evaluator property.

czi-fsisenda · 2026-06-30T22:44:04Z

+        "required": ["id", "type"],
+        "properties": {
+          "id": { "type": "string", "minLength": 1 },
+          "type": { "enum": ["llm", "api"] },


[P1] Should we allow for a "function" type too? If there's a common step in a several evals that is not an llm or api call, we could still represent it as a step, e.g. Flesch-Kincaid.

czi-fsisenda · 2026-06-30T22:46:31Z

+            "additionalProperties": false,
+            "required": ["provider", "name"],
+            "properties": {
+              "provider": { "enum": ["google", "openai", "anthropic"] },


[P1] Should we include "custom" even if we haven't implemented support yet?

czi-fsisenda · 2026-06-30T23:23:18Z

    },
    "expected": {
-      "complexity_level": "slightly_complex"
+      "complexity_score": "slightly_complex"


[P2] Common outputs or output components for evaluator suites too?

czi-fsisenda · 2026-06-30T23:27:46Z

  "preprocessing": [
    {
      "id": "fk_score",
+      "type": "computation",


[P1] Oh! We do have a "function" step. What is the benefit in having a distinction between step and preprocessing?

czi-fsisenda · 2026-06-30T23:45:23Z

+
+  2. Cross-file — rules a schema cannot express because they span files:
+       - referenced files actually exist ($ref schemas, prompt files, fixtures)
+       - each declared sha256 matches the prompt file on disk (drift tripwire)


[P2] An alternative approach could be to generate a flat file from all the config jsons and have that represent a version of the eval. If the generated file changes, then we have a new version.
The generation step can be a proxy for a number of the checks. Generation fails if files are missing or are invalid.

czi-fsisenda · 2026-06-30T23:56:05Z

+       - referenced files actually exist ($ref schemas, prompt files, fixtures)
+       - each declared sha256 matches the prompt file on disk (drift tripwire)
+       - placeholders declared in config line up with the {vars} in the prompts
+       - system prompts carry no user-input placeholders


[P0] Makes logical sense, but is it a necessary constraint? What if there is some valid reason to have a dynamic system prompt? This constraint may be too eager.

czi-fsisenda · 2026-07-01T00:00:16Z

+       - each declared sha256 matches the prompt file on disk (drift tripwire)
+       - placeholders declared in config line up with the {vars} in the prompts
+       - system prompts carry no user-input placeholders
+       - no obsolete format-instruction placeholders survive anywhere


[P2] Good for us to have automated enforcement for established patterns. But also wondering if this is too strict; having us lose flexibility. Could a warning work rather than a failure?

adnanrhussain marked this pull request as draft June 25, 2026 23:48

chore: add eval-config validation check

9ce2973

adnanrhussain force-pushed the ahussain/eval-config-checks branch from 7dff249 to 9ce2973 Compare June 25, 2026 23:54

feat: schema-driven eval-config validation with shared config schema

1b2378a

adnanrhussain mentioned this pull request Jun 29, 2026

feat: add eval-schemas, eval-fixtures, eval-notebook checks #127

Open

adnanrhussain marked this pull request as ready for review June 29, 2026 19:35

adnanrhussain requested review from a team, Copilot and czi-fsisenda June 29, 2026 19:35

Copilot started reviewing on behalf of adnanrhussain June 29, 2026 19:36 View session

Copilot AI reviewed Jun 29, 2026

View reviewed changes

Comment thread scripts/checks/eval_config.py

Comment thread scripts/checks/eval_config.py

Comment thread scripts/checks/eval_config.py

Comment thread scripts/checks/eval_config.py Outdated

Comment thread evals/purpose_evaluator.ipynb Outdated

fix: harden eval-config (guard schema/prompt reads, hash raw bytes); …

8f5109c

…fix notebook typo

czi-fsisenda approved these changes Jul 1, 2026

View reviewed changes

Uh oh!

Conversation

adnanrhussain commented Jun 25, 2026

What

Rules enforced

It immediately caught a real leftover

Deferred (noted in scripts/README.md)

Uh oh!

codecov Bot commented Jun 29, 2026

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

czi-fsisenda left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Deferred (noted in `scripts/README.md`)