feat(eval): add eval packet, harness change manifest, and CI attribution scorecard schemas#128
Closed
hummbl-dev wants to merge 2 commits into
Closed
feat(eval): add eval packet, harness change manifest, and CI attribution scorecard schemas#128hummbl-dev wants to merge 2 commits into
hummbl-dev wants to merge 2 commits into
Conversation
…ion scorecard schemas Implements issue #122: Eval and CI Observability Pack for agent harnesses. New files: - schemas/eval_packet.schema.json - schemas/harness_change_manifest.schema.json - schemas/ci_attribution_scorecard.schema.json - scripts/validate_eval_schemas.py (stdlib-only) - fixtures/eval/valid_eval_packet.json - fixtures/eval/valid_harness_change_manifest.json - fixtures/eval/valid_ci_attribution_scorecard.json Schemas designed per issue #122 spec: 1. Eval packet: domain, target agent, traps, jurors, audit rules, scoring rubric, fallback policy, trace requirements, evidence links, pass/fail policy 2. Harness-change manifest: changed component, failure evidence, root-cause hypothesis, expected fix, expected regressions, rollback plan, verifier, evaluation set, outcome, next-round verdict 3. CI attribution scorecard: PR, repo, executor agent, model, harness, tool access level, change category, CI result, duration, retry count, attribution confidence Closes #122 Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds three JSON Schemas (eval packet, harness change manifest, CI attribution scorecard) with corresponding valid JSON fixtures, and a Python CLI script ( ChangesEval and CI Observability Pack
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as validate_eval_schemas.py
participant Validator as VALIDATORS dispatch
participant Payload as JSON File
User->>CLI: run with schema_type, file_path, --strict
CLI->>Payload: load JSON payload
CLI->>Validator: dispatch(schema_type, payload)
Validator->>Validator: check required fields, enums, consistency
Validator-->>CLI: result (valid, violations, counts)
CLI-->>User: print JSON result, exit code
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds an “eval & CI observability pack” to arbiter by introducing three JSON Schemas (eval packet, harness-change manifest, CI attribution scorecard), a stdlib-only validator script for them, and corresponding valid fixture examples—aligning with the advisory/fixture-oriented scope described in issue #122.
Changes:
- Introduces 3 JSON Schema definitions under
schemas/for eval packets, harness-change manifests, and CI attribution scorecards. - Adds
scripts/validate_eval_schemas.pyto validate required fields/enums and some cross-field consistency checks (with--strictmode). - Adds 3 “known-good” fixtures under
fixtures/eval/(one per schema).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/validate_eval_schemas.py | Adds a stdlib-only validator for the three new schema lanes. |
| schemas/eval_packet.schema.json | Defines the Eval Packet schema (traps, jurors, audit rules, rubric, policies). |
| schemas/harness_change_manifest.schema.json | Defines the Harness Change Manifest schema (component change + evidence + rollback + outcome). |
| schemas/ci_attribution_scorecard.schema.json | Defines the CI Attribution Scorecard schema (per-PR entries + summary). |
| fixtures/eval/valid_eval_packet.json | Provides a valid eval packet example fixture. |
| fixtures/eval/valid_harness_change_manifest.json | Provides a valid harness change manifest example fixture. |
| fixtures/eval/valid_ci_attribution_scorecard.json | Provides a valid CI attribution scorecard example fixture. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+81
to
+85
| dims = rubric.get("dimensions", []) | ||
| if isinstance(dims, list) and len(dims) > 0: | ||
| dim_weight_sum = sum(d.get("weight", 0) for d in dims if isinstance(d, dict)) | ||
| if dim_weight_sum > 1.01: | ||
| violations.append(_v("G-RUBRIC-WEIGHTS-NORMALIZED", f"dimension weights sum to {dim_weight_sum}, must not exceed 1.0")) |
Comment on lines
+136
to
+140
| entries = payload.get("entries", []) | ||
| if not isinstance(entries, list): | ||
| violations.append(_v("G-ENTRIES-PRESENT", "entries must be a list")) | ||
| entries = [] | ||
|
|
Comment on lines
+32
to
+35
| "traps": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "object", |
Comment on lines
+47
to
+50
| "jurors": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "object", |
Comment on lines
+29
to
+32
| "entries": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "object", |
Comment on lines
+1
to
+5
| { | ||
| "schema_version": "0.1", | ||
| "packet_id": "eval-001", | ||
| "domain": "code_review_safety", | ||
| "target_agent": "devin", |
Comment on lines
+62
to
+80
| jurors = payload.get("jurors", []) | ||
| if not isinstance(jurors, list) or len(jurors) == 0: | ||
| violations.append(_v("G-JURORS-PRESENT", "jurors must be a non-empty list")) | ||
| else: | ||
| total_weight = 0 | ||
| for i, juror in enumerate(jurors): | ||
| if not isinstance(juror, dict): | ||
| violations.append(_v("G-JURORS-PRESENT", f"jurors[{i}] must be an object")) | ||
| continue | ||
| if juror.get("juror_type") not in VALID_JUROR_TYPES: | ||
| violations.append(_v("G-JUROR-TYPE-VALID", f"jurors[{i}] invalid juror_type: {juror.get('juror_type')}")) | ||
| w = juror.get("weight", 0) | ||
| if isinstance(w, (int, float)): | ||
| total_weight += w | ||
| if total_weight > 1.01: | ||
| violations.append(_v("G-JUROR-WEIGHTS-NORMALIZED", f"juror weights sum to {total_weight}, must not exceed 1.0")) | ||
|
|
||
| rubric = payload.get("scoring_rubric", {}) | ||
| if isinstance(rubric, dict): |
Comment on lines
+169
to
+181
| def main(argv: list[str] | None = None) -> int: | ||
| parser = argparse.ArgumentParser(description=__doc__) | ||
| parser.add_argument("schema_type", choices=list(VALIDATORS.keys()), help="Schema type to validate against") | ||
| parser.add_argument("file_path", help="Path to a JSON file") | ||
| parser.add_argument("--strict", action="store_true", help="Exit non-zero on errors") | ||
| args = parser.parse_args(argv) | ||
|
|
||
| with Path(args.file_path).open("r", encoding="utf-8") as f: | ||
| payload = json.load(f) | ||
|
|
||
| result = VALIDATORS[args.schema_type](payload) | ||
| print(json.dumps(result, indent=2, sort_keys=True)) | ||
| return 1 if args.strict and not result["valid"] else 0 |
3 tasks
hummbl-dev
added a commit
that referenced
this pull request
Jul 4, 2026
) * feat(eval): add eval packet, harness change manifest, and CI attribution scorecard schemas Implements issue #122: Eval and CI Observability Pack for agent harnesses. New files: - schemas/eval_packet.schema.json - schemas/harness_change_manifest.schema.json - schemas/ci_attribution_scorecard.schema.json - scripts/validate_eval_schemas.py (stdlib-only) - fixtures/eval/valid_eval_packet.json - fixtures/eval/valid_harness_change_manifest.json - fixtures/eval/valid_ci_attribution_scorecard.json Schemas designed per issue #122 spec: 1. Eval packet: domain, target agent, traps, jurors, audit rules, scoring rubric, fallback policy, trace requirements, evidence links, pass/fail policy 2. Harness-change manifest: changed component, failure evidence, root-cause hypothesis, expected fix, expected regressions, rollback plan, verifier, evaluation set, outcome, next-round verdict 3. CI attribution scorecard: PR, repo, executor agent, model, harness, tool access level, change category, CI result, duration, retry count, attribution confidence Closes #122 Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor: reduce cyclomatic complexity in validate_eval_schemas.py Extract validation logic into small helper functions to bring all functions from grade E/C down to grade A/B. Fixes self-grade CI failure on PR #128 (score was 65, threshold 70). - validate_eval_packet: CC 32 -> B (extracted trap, juror, rubric, fallback, pass_fail helpers) - validate_harness_change: CC 16 -> A (extracted changed_component, rollback helpers, use _check_enum) - validate_ci_scorecard: CC 16 -> A (extracted entry, summary helpers) - Added shared _check_enum, _check_nonempty_list, _result helpers --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Extract validation logic into small helper functions to bring all functions from grade E/C down to grade A/B. Fixes self-grade CI failure on PR #128 (score was 65, threshold 70). - validate_eval_packet: CC 32 -> B (extracted trap, juror, rubric, fallback, pass_fail helpers) - validate_harness_change: CC 16 -> A (extracted changed_component, rollback helpers, use _check_enum) - validate_ci_scorecard: CC 16 -> A (extracted entry, summary helpers) - Added shared _check_enum, _check_nonempty_list, _result helpers
Owner
Author
|
CI is now green (all tests + self-grade pass). The cyclomatic complexity fix from #129 has been cherry-picked into this branch. Ready for review/merge. |
Owner
Author
|
Closing as duplicate — all changes (schemas, fixtures, and complexity refactor) were merged to main via #129. The Copilot review comments are being addressed in a follow-up PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@schemas/ci_attribution_scorecard.schema.json`:
- Around line 57-77: The nested value objects inside by_agent and by_category
are still open-ended, unlike the other schema objects. Update the object
definitions in ci_attribution_scorecard.schema.json so the additionalProperties
blocks for by_agent and by_category also explicitly disallow extra keys,
matching the closed shape used elsewhere in the schema.
In `@schemas/eval_packet.schema.json`:
- Around line 119-128: The pass_fail_policy schema and validator currently allow
mode best_of_n without a valid n value. Update eval_packet.schema.json to add an
if/then conditional on pass_fail_policy so that when mode is best_of_n, n
becomes required and must be a positive integer, and mirror the same check in
_validate_pass_fail to reject null or missing n for that mode.
- Around line 32-60: The schema for eval packets currently allows empty traps
and jurors arrays, which conflicts with the validator contract. Update the traps
and jurors array definitions in eval_packet.schema.json to require at least one
item, aligning them with the non-empty checks enforced by
scripts/validate_eval_schemas.py (_check_nonempty_list and the G-TRAPS-PRESENT /
G-JURORS-PRESENT gates). Keep the existing item object constraints unchanged and
apply the fix directly to the traps and jurors properties.
In `@scripts/validate_eval_schemas.py`:
- Around line 16-29: The validator in validate_eval_schemas.py is duplicating
schema rules with VALID_* constants instead of fully enforcing the schemas, so
update the schema-checking flow to stay aligned with schemas/*.schema.json. Use
the existing helpers in validate_eval_schemas.py to add at least
unexpected-property detection for additionalProperties: false, and cover the
missing field/range constraints such as per-item weight bounds, date/date-time
formats, and numeric minimums. If full schema validation remains out of scope,
add a lightweight test that compares the VALID_* sets against the schema enum
arrays so the constants cannot drift from the source schemas.
- Around line 40-45: Explicit nulls are bypassing enum validation in _check_enum
because it skips checks when the field value is None, so required enum fields
can pass under strict validation. Update _check_enum in validate_eval_schemas.py
to treat a present key with null as invalid for enum-backed fields, and make
sure it still works with _check_required so fields like status, outcome,
next_round_verdict, ci_result, harness, and change_category are rejected when
explicitly set to null.
- Around line 232-234: The file loading in validate_eval_schemas.py has no
graceful handling for missing files or invalid JSON. Update the code around the
Path(args.file_path).open and json.load call to catch file/JSON exceptions, then
surface a clean CLI-style error and exit failure instead of letting an unhandled
traceback escape. Keep the fix localized near the existing payload parsing logic
so the main validation flow can continue to use payload only after successful
parsing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d9643ff9-ea07-4240-9e80-a6413e933fec
📒 Files selected for processing (7)
fixtures/eval/valid_ci_attribution_scorecard.jsonfixtures/eval/valid_eval_packet.jsonfixtures/eval/valid_harness_change_manifest.jsonschemas/ci_attribution_scorecard.schema.jsonschemas/eval_packet.schema.jsonschemas/harness_change_manifest.schema.jsonscripts/validate_eval_schemas.py
Comment on lines
+57
to
+77
| "by_agent": { | ||
| "type": "object", | ||
| "additionalProperties": { | ||
| "type": "object", | ||
| "properties": { | ||
| "prs": { "type": "integer" }, | ||
| "pass_rate": { "type": "number" }, | ||
| "avg_duration": { "type": "number" } | ||
| } | ||
| } | ||
| }, | ||
| "by_category": { | ||
| "type": "object", | ||
| "additionalProperties": { | ||
| "type": "object", | ||
| "properties": { | ||
| "prs": { "type": "integer" }, | ||
| "pass_rate": { "type": "number" } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
Nested by_agent/by_category value objects lack additionalProperties: false.
Every other object in these three schemas is closed with additionalProperties: false. These two nested definitions are the only exception, allowing arbitrary extra keys to slip in unchecked.
🛠️ Proposed fix
"by_agent": {
"type": "object",
"additionalProperties": {
"type": "object",
+ "additionalProperties": false,
+ "required": ["prs", "pass_rate", "avg_duration"],
"properties": {
"prs": { "type": "integer" },
"pass_rate": { "type": "number" },
"avg_duration": { "type": "number" }
}
}
},
"by_category": {
"type": "object",
"additionalProperties": {
"type": "object",
+ "additionalProperties": false,
+ "required": ["prs", "pass_rate"],
"properties": {
"prs": { "type": "integer" },
"pass_rate": { "type": "number" }
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "by_agent": { | |
| "type": "object", | |
| "additionalProperties": { | |
| "type": "object", | |
| "properties": { | |
| "prs": { "type": "integer" }, | |
| "pass_rate": { "type": "number" }, | |
| "avg_duration": { "type": "number" } | |
| } | |
| } | |
| }, | |
| "by_category": { | |
| "type": "object", | |
| "additionalProperties": { | |
| "type": "object", | |
| "properties": { | |
| "prs": { "type": "integer" }, | |
| "pass_rate": { "type": "number" } | |
| } | |
| } | |
| } | |
| "by_agent": { | |
| "type": "object", | |
| "additionalProperties": { | |
| "type": "object", | |
| "additionalProperties": false, | |
| "required": ["prs", "pass_rate", "avg_duration"], | |
| "properties": { | |
| "prs": { "type": "integer" }, | |
| "pass_rate": { "type": "number" }, | |
| "avg_duration": { "type": "number" } | |
| } | |
| } | |
| }, | |
| "by_category": { | |
| "type": "object", | |
| "additionalProperties": { | |
| "type": "object", | |
| "additionalProperties": false, | |
| "required": ["prs", "pass_rate"], | |
| "properties": { | |
| "prs": { "type": "integer" }, | |
| "pass_rate": { "type": "number" } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@schemas/ci_attribution_scorecard.schema.json` around lines 57 - 77, The
nested value objects inside by_agent and by_category are still open-ended,
unlike the other schema objects. Update the object definitions in
ci_attribution_scorecard.schema.json so the additionalProperties blocks for
by_agent and by_category also explicitly disallow extra keys, matching the
closed shape used elsewhere in the schema.
Comment on lines
+32
to
+60
| "traps": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "required": ["trap_id", "description", "expected_behavior", "risk_class"], | ||
| "properties": { | ||
| "trap_id": { "type": "string", "minLength": 1 }, | ||
| "description": { "type": "string", "minLength": 1 }, | ||
| "expected_behavior": { "type": "string", "enum": ["block", "warn", "escalate", "refuse", "isolate"] }, | ||
| "risk_class": { "type": "string", "enum": ["prompt_injection", "social_engineering", "scope_creep", "secret_exfil", "destructive_action", "hallucination"] }, | ||
| "input_text": { "type": "string" } | ||
| } | ||
| } | ||
| }, | ||
| "jurors": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "required": ["juror_id", "juror_type", "weight"], | ||
| "properties": { | ||
| "juror_id": { "type": "string", "minLength": 1 }, | ||
| "juror_type": { "type": "string", "enum": ["human", "automated", "model", "rule_based"] }, | ||
| "weight": { "type": "number", "minimum": 0, "maximum": 1 }, | ||
| "persona": { "type": "string" } | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add minItems: 1 to traps and jurors to match validator contract.
The validator in scripts/validate_eval_schemas.py requires non-empty traps and jurors lists (_check_nonempty_list, gates G-TRAPS-PRESENT/G-JURORS-PRESENT), but this schema allows empty arrays. A payload with "traps": [] would pass JSON Schema validation yet fail the hand-rolled validator — the two enforcement layers disagree on what's valid.
🛠️ Proposed fix
"traps": {
"type": "array",
+ "minItems": 1,
"items": { "jurors": {
"type": "array",
+ "minItems": 1,
"items": {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "traps": { | |
| "type": "array", | |
| "items": { | |
| "type": "object", | |
| "additionalProperties": false, | |
| "required": ["trap_id", "description", "expected_behavior", "risk_class"], | |
| "properties": { | |
| "trap_id": { "type": "string", "minLength": 1 }, | |
| "description": { "type": "string", "minLength": 1 }, | |
| "expected_behavior": { "type": "string", "enum": ["block", "warn", "escalate", "refuse", "isolate"] }, | |
| "risk_class": { "type": "string", "enum": ["prompt_injection", "social_engineering", "scope_creep", "secret_exfil", "destructive_action", "hallucination"] }, | |
| "input_text": { "type": "string" } | |
| } | |
| } | |
| }, | |
| "jurors": { | |
| "type": "array", | |
| "items": { | |
| "type": "object", | |
| "additionalProperties": false, | |
| "required": ["juror_id", "juror_type", "weight"], | |
| "properties": { | |
| "juror_id": { "type": "string", "minLength": 1 }, | |
| "juror_type": { "type": "string", "enum": ["human", "automated", "model", "rule_based"] }, | |
| "weight": { "type": "number", "minimum": 0, "maximum": 1 }, | |
| "persona": { "type": "string" } | |
| } | |
| } | |
| }, | |
| "traps": { | |
| "type": "array", | |
| "minItems": 1, | |
| "items": { | |
| "type": "object", | |
| "additionalProperties": false, | |
| "required": ["trap_id", "description", "expected_behavior", "risk_class"], | |
| "properties": { | |
| "trap_id": { "type": "string", "minLength": 1 }, | |
| "description": { "type": "string", "minLength": 1 }, | |
| "expected_behavior": { "type": "string", "enum": ["block", "warn", "escalate", "refuse", "isolate"] }, | |
| "risk_class": { "type": "string", "enum": ["prompt_injection", "social_engineering", "scope_creep", "secret_exfil", "destructive_action", "hallucination"] }, | |
| "input_text": { "type": "string" } | |
| } | |
| } | |
| }, | |
| "jurors": { | |
| "type": "array", | |
| "minItems": 1, | |
| "items": { | |
| "type": "object", | |
| "additionalProperties": false, | |
| "required": ["juror_id", "juror_type", "weight"], | |
| "properties": { | |
| "juror_id": { "type": "string", "minLength": 1 }, | |
| "juror_type": { "type": "string", "enum": ["human", "automated", "model", "rule_based"] }, | |
| "weight": { "type": "number", "minimum": 0, "maximum": 1 }, | |
| "persona": { "type": "string" } | |
| } | |
| } | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@schemas/eval_packet.schema.json` around lines 32 - 60, The schema for eval
packets currently allows empty traps and jurors arrays, which conflicts with the
validator contract. Update the traps and jurors array definitions in
eval_packet.schema.json to require at least one item, aligning them with the
non-empty checks enforced by scripts/validate_eval_schemas.py
(_check_nonempty_list and the G-TRAPS-PRESENT / G-JURORS-PRESENT gates). Keep
the existing item object constraints unchanged and apply the fix directly to the
traps and jurors properties.
Comment on lines
+119
to
+128
| "pass_fail_policy": { | ||
| "type": "object", | ||
| "additionalProperties": false, | ||
| "required": ["mode", "threshold"], | ||
| "properties": { | ||
| "mode": { "type": "string", "enum": ["all_or_nothing", "weighted", "best_of_n"] }, | ||
| "threshold": { "type": "number" }, | ||
| "n": { "type": ["integer", "null"], "minimum": 1 } | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
n isn't required when mode is best_of_n.
A payload can set "mode": "best_of_n" while leaving n as null, and neither this schema nor _validate_pass_fail in the validator catches it. Consider an if/then conditional requiring n (and a sensible minimum) when mode is best_of_n.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@schemas/eval_packet.schema.json` around lines 119 - 128, The pass_fail_policy
schema and validator currently allow mode best_of_n without a valid n value.
Update eval_packet.schema.json to add an if/then conditional on pass_fail_policy
so that when mode is best_of_n, n becomes required and must be a positive
integer, and mirror the same check in _validate_pass_fail to reject null or
missing n for that mode.
Comment on lines
+16
to
+29
| VALID_STATUSES = {"draft", "candidate", "active", "retired"} | ||
| VALID_TRAP_BEHAVIORS = {"block", "warn", "escalate", "refuse", "isolate"} | ||
| VALID_RISK_CLASSES = {"prompt_injection", "social_engineering", "scope_creep", "secret_exfil", "destructive_action", "hallucination"} | ||
| VALID_JUROR_TYPES = {"human", "automated", "model", "rule_based"} | ||
| VALID_CHECK_TYPES = {"output_format", "safety_boundary", "attribution_correctness", "evidence_presence", "receipt_validity", "no_hallucination"} | ||
| VALID_PASS_FAIL_MODES = {"all_or_nothing", "weighted", "best_of_n"} | ||
| VALID_FALLBACK_ACTIONS = {"block", "retry", "escalate", "degrade"} | ||
| VALID_CHANGE_TYPES = {"add", "modify", "remove", "fix", "refactor"} | ||
| VALID_COMPONENT_TYPES = {"analyzer", "scorer", "attribution", "dashboard", "ci_integration", "governance", "tooling"} | ||
| VALID_OUTCOMES = {"succeeded", "failed", "partial", "reverted", "pending"} | ||
| VALID_VERDICTS = {"promote", "hold", "revert", "iterate", "block"} | ||
| VALID_CI_RESULTS = {"passed", "failed", "flaky", "cancelled", "skipped"} | ||
| VALID_HARNESSES = {"github_actions", "local", "ci_runner", "manual"} | ||
| VALID_CHANGE_CATEGORIES = {"feature", "bugfix", "refactor", "docs", "test", "config", "governance", "schema"} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy lift
Validator duplicates schema constraints by hand and covers only a subset of them.
This script reimplements required-field/enum checks as separate Python constants rather than validating against the actual schemas/*.schema.json files, and it's missing several constraints those schemas encode: additionalProperties: false (unexpected keys pass silently), per-item weight bounds 0-1 on jurors/dimensions (only the aggregate sum is checked), format: date/date-time, and numeric ranges like duration_seconds/pr_number minimums. Two sources of truth (schema files and VALID_* sets) can drift silently, and the checked cases here don't fully back the PR's "strict validation" claim.
Given the stated stdlib-only constraint, consider either using the stdlib-only path to at least flag unexpected properties (mirroring additionalProperties: false), or add a lightweight test asserting the VALID_* sets stay in sync with the schema enum arrays.
Also applies to: 118-216
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/validate_eval_schemas.py` around lines 16 - 29, The validator in
validate_eval_schemas.py is duplicating schema rules with VALID_* constants
instead of fully enforcing the schemas, so update the schema-checking flow to
stay aligned with schemas/*.schema.json. Use the existing helpers in
validate_eval_schemas.py to add at least unexpected-property detection for
additionalProperties: false, and cover the missing field/range constraints such
as per-item weight bounds, date/date-time formats, and numeric minimums. If full
schema validation remains out of scope, add a lightweight test that compares the
VALID_* sets against the schema enum arrays so the constants cannot drift from
the source schemas.
Comment on lines
+40
to
+45
| def _check_enum(payload: dict, field: str, valid: set, gate: str, label: str) -> list: | ||
| """Validate that payload[field] is in the valid set, if present.""" | ||
| val = payload.get(field) | ||
| if val is not None and val not in valid: | ||
| return [_v(gate, f"{label} invalid {field}: {val}")] | ||
| return [] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Explicit null values silently bypass enum checks.
_check_enum only flags a value when val is not None. Combined with _check_required's "key present" check (which treats "status": null as satisfying the required field), a payload can set required enum fields (status, outcome, next_round_verdict, ci_result, harness, change_category) to explicit null and still report valid: true under --strict.
🐛 Proposed fix
def _check_enum(payload: dict, field: str, valid: set, gate: str, label: str) -> list:
"""Validate that payload[field] is in the valid set, if present."""
- val = payload.get(field)
- if val is not None and val not in valid:
+ if field not in payload:
+ return []
+ val = payload[field]
+ if val not in valid:
return [_v(gate, f"{label} invalid {field}: {val}")]
return []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _check_enum(payload: dict, field: str, valid: set, gate: str, label: str) -> list: | |
| """Validate that payload[field] is in the valid set, if present.""" | |
| val = payload.get(field) | |
| if val is not None and val not in valid: | |
| return [_v(gate, f"{label} invalid {field}: {val}")] | |
| return [] | |
| def _check_enum(payload: dict, field: str, valid: set, gate: str, label: str) -> list: | |
| """Validate that payload[field] is in the valid set, if present.""" | |
| if field not in payload: | |
| return [] | |
| val = payload[field] | |
| if val not in valid: | |
| return [_v(gate, f"{label} invalid {field}: {val}")] | |
| return [] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/validate_eval_schemas.py` around lines 40 - 45, Explicit nulls are
bypassing enum validation in _check_enum because it skips checks when the field
value is None, so required enum fields can pass under strict validation. Update
_check_enum in validate_eval_schemas.py to treat a present key with null as
invalid for enum-backed fields, and make sure it still works with
_check_required so fields like status, outcome, next_round_verdict, ci_result,
harness, and change_category are rejected when explicitly set to null.
Comment on lines
+232
to
+234
| with Path(args.file_path).open("r", encoding="utf-8") as f: | ||
| payload = json.load(f) | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
No error handling around file read/JSON parse.
A missing file or malformed JSON raises an unhandled traceback instead of a clean CLI error. Minor for a CI-only tool, but worth a friendlier failure path.
🛠️ Proposed fix
- with Path(args.file_path).open("r", encoding="utf-8") as f:
- payload = json.load(f)
+ try:
+ with Path(args.file_path).open("r", encoding="utf-8") as f:
+ payload = json.load(f)
+ except (OSError, json.JSONDecodeError) as exc:
+ print(json.dumps({"valid": False, "error": str(exc)}, indent=2))
+ return 1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| with Path(args.file_path).open("r", encoding="utf-8") as f: | |
| payload = json.load(f) | |
| try: | |
| with Path(args.file_path).open("r", encoding="utf-8") as f: | |
| payload = json.load(f) | |
| except (OSError, json.JSONDecodeError) as exc: | |
| print(json.dumps({"valid": False, "error": str(exc)}, indent=2)) | |
| return 1 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/validate_eval_schemas.py` around lines 232 - 234, The file loading in
validate_eval_schemas.py has no graceful handling for missing files or invalid
JSON. Update the code around the Path(args.file_path).open and json.load call to
catch file/JSON exceptions, then surface a clean CLI-style error and exit
failure instead of letting an unhandled traceback escape. Keep the fix localized
near the existing payload parsing logic so the main validation flow can continue
to use payload only after successful parsing.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
scripts/validate_eval_schemas.py— stdlib-only validator for all 3 schema typesSource status
Canon status
Privacy impact
Safety impact
Issues
Closes: #122
References: hummbl-dev/founder-mode#1190 (parent routing issue)
Review notes
Schema 1: Eval Packet
Per issue #122 spec: domain, target agent, traps, jurors, audit rules, scoring rubric, fallback policy, trace requirements, evidence links, pass/fail policy.
Validator gates:
Schema 2: Harness Change Manifest
Per issue #122 spec: changed component, failure evidence, root-cause hypothesis, expected fix, expected regressions, rollback plan, verifier, evaluation set, outcome, next-round verdict.
Validator gates:
Schema 3: CI Attribution Scorecard
Per issue #122 spec: PR, repo, executor agent, model, harness, change category, CI result, duration, retry count, attribution confidence.
Validator gates:
Local verification
Stdlib-only compliance
Per AGENTS.md: "Core is stdlib-only". The validator uses only stdlib (json, pathlib, argparse, typing).
Generated with Devin