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.

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.

You are right, but this isnt for the inputs. Its to describe which grades an eval is supported for. All our evals should be for K-12, but as we incrementally iterate and launch, the grade support matrix will fillin over cycles. For example, the new feedback evals are supported for grades 8-9 only. This field will help clarify that support

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.

required and minLength do different jobs: required enforces the key is present, minLength: 1 enforces the value is non-empty. With required alone, {"id": "", "name": ""} validates fine - so minLength: 1 is what actually rejects empty strings (it's the standard JSON Schema idiom for "non-empty," not an endorsement of 1-char values). allow_empty isn't a JSON Schema keyword. So we need both, and I'd keep them as-is.

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.

no strong opinion on either but I am leaning on string due to K and making it more readable

edit: see response on eval support vs input

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.

we do have a computation for for pre-processing. eventually we can support all across pre/steps/post, etc

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?

we can add in a followup

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?

nothing specific seperates them at the moment, if this shows up in the steps, we will merge these

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.

Really like this, especially the versioning angle, let me come back to this

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.

possibly, but its best practice for the system prompt to be static, and all user inputs are in the user prompt. Currently our prompts are prone to prompt injection due to this, and need to be cleaned-up. Once enforced and the pattern reaches stability, we can relax this again if needed

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?

the prompts shouldnt take an opinionated approach for format instruction placements in the prompts. Our users may not want it. This is not required, and more of a remnant that we need to cleanup

…fix notebook typo

…onfig

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

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

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

czi-fsisenda approved these changes Jul 1, 2026

View reviewed changes

adnanrhussain force-pushed the ahussain/nbstripout branch from 342b46c to 623fd4e Compare July 16, 2026 19:29

Base automatically changed from ahussain/nbstripout to main July 16, 2026 19:31

adnanrhussain force-pushed the ahussain/eval-config-checks branch from 8f5109c to 71a6492 Compare July 16, 2026 20:03

adnanrhussain added 4 commits July 16, 2026 16:39

chore: add eval-config validation check

5e80ea4

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

9bbe1dd

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

edbf60a

…fix notebook typo

fix: drop stale unused 'n' placeholder from math-question-alignment c…

8c89bbb

…onfig

adnanrhussain force-pushed the ahussain/eval-config-checks branch from 71a6492 to 8c89bbb Compare July 16, 2026 23:39

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!

adnanrhussain Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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`)

adnanrhussain Jul 16, 2026 •

edited

Loading