chore: add eval-config validation check #120
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,170 @@ | ||
| { | ||
| "$schema": "https://json-schema.org/draft/2020-12/schema", | ||
| "$id": "config.schema.json", | ||
| "title": "Evaluator config", | ||
| "description": "Shared contract for evals/**/config.json. One living schema, referenced by each config via its $schema field. Strict: additionalProperties is false at every level, so any new field must be added here deliberately.", | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "required": ["$schema", "evaluator", "input_schema", "steps", "output_schema"], | ||
| "properties": { | ||
| "$schema": { "type": "string" }, | ||
| "evaluator": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "required": ["id", "name", "description", "supported_grades"], | ||
| "properties": { | ||
| "id": { "type": "string", "minLength": 1 }, | ||
| "name": { "type": "string", "minLength": 1 }, | ||
| "description": { "type": "string", "minLength": 1 }, | ||
|
Comment on lines
+16
to
+18
Contributor
There was a problem hiding this comment. [P1?]
Contributor
Author
There was a problem hiding this comment.
Contributor
There was a problem hiding this comment. For input fields,
|
||
| "supported_grades": { | ||
| "type": "array", | ||
| "minItems": 1, | ||
| "items": { | ||
| "enum": ["K", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12"] | ||
|
Contributor
There was a problem hiding this comment. [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.
Contributor
Author
There was a problem hiding this comment. 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
Contributor
There was a problem hiding this comment. I think I saw KG support pre-K as a grade. |
||
| } | ||
| } | ||
| } | ||
| }, | ||
| "input_schema": { "type": "object" }, | ||
| "output_schema": { "type": "object" }, | ||
| "preprocessing": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "required": ["id", "type", "kind"], | ||
| "properties": { | ||
| "id": { "type": "string", "minLength": 1 }, | ||
| "type": { "enum": ["api", "computation"] }, | ||
| "kind": { "type": "string", "minLength": 1 }, | ||
| "description": { "type": "string" }, | ||
| "input": { "type": "string" }, | ||
| "output": { "type": "string" }, | ||
| "implementation": { | ||
| "type": "object", | ||
| "minProperties": 1, | ||
| "additionalProperties": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "required": ["library", "function"], | ||
| "properties": { | ||
| "library": { "type": "string", "minLength": 1 }, | ||
| "function": { "type": "string", "minLength": 1 }, | ||
| "post_transform": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "required": ["type"], | ||
| "properties": { | ||
| "type": { "enum": ["round"] }, | ||
| "precision": { "type": "integer" } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "endpoint": { "type": "string" }, | ||
| "params": { "type": "object" }, | ||
| "auth": { "type": "string" }, | ||
| "pagination": { "type": "string" } | ||
| }, | ||
| "allOf": [ | ||
| { | ||
| "if": { "properties": { "type": { "const": "api" } } }, | ||
| "then": { "required": ["endpoint", "params", "auth"] } | ||
| }, | ||
| { | ||
| "if": { "properties": { "type": { "const": "computation" } } }, | ||
| "then": { "required": ["implementation", "input", "output"] } | ||
| } | ||
| ] | ||
| } | ||
| }, | ||
| "steps": { | ||
| "type": "array", | ||
| "minItems": 1, | ||
| "items": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "required": ["id", "type"], | ||
| "properties": { | ||
| "id": { "type": "string", "minLength": 1 }, | ||
| "type": { "enum": ["llm", "api"] }, | ||
|
Contributor
There was a problem hiding this comment. [P1] Should we allow for a
Contributor
Author
There was a problem hiding this comment. we do have a |
||
| "description": { "type": "string" }, | ||
| "prompt": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "required": ["messages", "placeholders"], | ||
| "properties": { | ||
| "messages": { | ||
| "type": "array", | ||
| "minItems": 1, | ||
| "items": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "required": ["role", "source_path"], | ||
| "properties": { | ||
|
Contributor
There was a problem hiding this comment. [P1] Newer models are dropping temperature, but most of the existing evals are using older models that have temperature as an attribute of the prompts step. Should we include
Contributor
Author
There was a problem hiding this comment. |
||
| "role": { "enum": ["system", "user", "assistant"] }, | ||
| "source_path": { "type": "string", "minLength": 1 }, | ||
| "sha256": { "type": "string", "pattern": "^[a-f0-9]{64}$" } | ||
| } | ||
| } | ||
| }, | ||
| "placeholders": { | ||
| "type": "object", | ||
| "additionalProperties": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "required": ["required", "source"], | ||
| "properties": { | ||
| "required": { "type": "boolean" }, | ||
| "source": { "type": "string" }, | ||
| "note": { "type": "string" } | ||
| } | ||
| } | ||
| } | ||
|
Comment on lines
+112
to
+124
Contributor
There was a problem hiding this comment. [P0] Can't placeholders can be inferred from the prompt templates? |
||
| } | ||
| }, | ||
| "model": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "required": ["provider", "name"], | ||
| "properties": { | ||
| "provider": { "enum": ["google", "openai", "anthropic"] }, | ||
|
Contributor
There was a problem hiding this comment. [P1] Should we include
Contributor
Author
There was a problem hiding this comment. we can add in a followup |
||
| "name": { "type": "string", "minLength": 1 }, | ||
| "alias": { "type": "string" } | ||
| } | ||
| }, | ||
| "generation": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "properties": { | ||
| "temperature": { "type": "number", "minimum": 0, "maximum": 1 } | ||
|
Contributor
There was a problem hiding this comment. Ah! Here's where temperature lives. |
||
| } | ||
| }, | ||
| "parser": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "required": ["kind"], | ||
| "properties": { | ||
| "kind": { "const": "structured_output" } | ||
| } | ||
| } | ||
| }, | ||
| "allOf": [ | ||
| { | ||
| "if": { "properties": { "type": { "const": "llm" } } }, | ||
| "then": { "required": ["prompt", "model", "parser"] } | ||
| } | ||
| ] | ||
| } | ||
| }, | ||
| "fixtures": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "properties": { | ||
| "path": { "type": "string" }, | ||
| "tolerance": { "type": "object" } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| { | ||
| "$schema": "https://json-schema.org/draft/2020-12/schema", | ||
| "$id": "fixtures.schema.json", | ||
| "title": "Evaluator fixtures", | ||
| "description": "Shared contract for evals/**/fixtures.json. Each case carries inputs and the expected label(s); per-evaluator input/expected shapes are validated against the evaluator's own input_schema/output_schema by the eval-fixtures check, not here.", | ||
| "type": "array", | ||
| "items": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "required": ["id", "input", "expected"], | ||
|
Contributor
There was a problem hiding this comment. [P1] The captures for the contract tests have: name (id), description, inputs, outputs, llm_captures. Should the fixtures replace the captures and include the additional fields?
Contributor
Author
There was a problem hiding this comment. This is a generic placeholder, the inputs and outputs are the same as eval specific config. Which properties should we also include here?
Contributor
There was a problem hiding this comment. We could add |
||
| "properties": { | ||
| "id": { "type": "string", "minLength": 1 }, | ||
| "description": { "type": "string" }, | ||
| "source": { "type": "string" }, | ||
| "input": { "type": "object" }, | ||
| "expected": { "type": "object" } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,10 @@ | ||
| { | ||
| "$schema": "../../../_schemas/config.schema.json", | ||
| "evaluator": { | ||
| "id": "feedback.productive_coaching_writing_feedback.is_appropriate_feedback", | ||
| "name": "Appropriate Feedback Feedback-Quality Evaluator", | ||
| "description": "Did the feedback correctly identify whether the student needed to revise? (Key features generated from rubric text; concept paper did not list explicit key feature items.)", | ||
| "supported_grades": ["8", "9"] | ||
|
Contributor
There was a problem hiding this comment. [P1] Grade isn't an input for the feedback evals. So it looks like
Contributor
Author
There was a problem hiding this comment. @aychi1 - Would have the best insight on grade selection for feedback data |
||
| }, | ||
| "input_schema": { | ||
| "$ref": "input_schema.json" | ||
|
Contributor
There was a problem hiding this comment. [P2] All the evals in a suite have the same input. Should we define that input once and reference it in all the evals?
Contributor
Author
There was a problem hiding this comment. yeah, I was thinking about that, but didnt want to overoptimize until we have the full feedback suite implemented |
||
|
|
@@ -12,9 +13,9 @@ | |
| "steps": [ | ||
| { | ||
| "id": "evaluate_is_appropriate_feedback", | ||
| "type": "llm", | ||
| "description": "Single-call LLM step that produces the binary EvaluatorOutput JSON.", | ||
| "prompt": { | ||
| "messages": [ | ||
| { | ||
| "role": "system", | ||
|
|
@@ -46,17 +47,14 @@ | |
| "temperature": 1 | ||
| }, | ||
| "parser": { | ||
| "kind": "structured_output" | ||
| } | ||
| } | ||
| ], | ||
| "output_schema": { | ||
| "$ref": "output_schema.json" | ||
| }, | ||
| "fixtures": { | ||
| "path": "fixtures.json" | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Saw your response below. Let's hear what Ariena says about how grade was determined for annotations.
I think I'm feeling more convinced that grade should be an input if the eval only works for some grade constraint. Counting on users checking an eval attribute seems like a hoping for best intentions where an explicit validation is possible.