Fix validate_json.py: parse all fields.yaml schemas; stop vacuous pass#8
Open
xwang4-svg wants to merge 1 commit into
Open
Fix validate_json.py: parse all fields.yaml schemas; stop vacuous pass#8xwang4-svg wants to merge 1 commit into
xwang4-svg wants to merge 1 commit into
Conversation
The validator only parsed the `field_categories:` schema, but /research emits
fields.yaml as `fields: {<category>: [{name, description, detail_level}]}`. That
shape was read as ZERO fields, so coverage defaulted to 100% and every result
JSON "passed" -- the validation step was effectively a no-op. `required` also
defaulted to False, so `valid` stayed True even when fields were missing.
- load_fields_yaml: accept field_categories / fields:{cat:[...]} / flat fields:[...]
plus a generic fallback; treat all fields as required when no `required:` marker
exists (explicit `required:` markers keep their opt-in semantics, unchanged).
- extract_json_fields: also descend into JSON keys named after declared categories.
- Apply to all 4 skill copies (research-en/zh, research-codex-en/zh).
- Add tests/test_validate_json_schemas.py regression test (guards all copies).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Problem
validate_json.pyis meant to enforce that each/research-deepresult JSON covers every field defined infields.yaml("Task is complete only after validation passes"). In practice it passed everything, due to two issues:1. Only the
field_categories:schema was parsed. But/research(per its ownSKILL.mdStep 4) emitsfields.yamlas:load_fields_yamlreaddata.get("field_categories", [])→[]→ 0 fields. With an empty field set,coverage_ratedefaults to100%andmissing_requiredis empty, so every JSON "passes" vacuously. (field_categoriesdoes not appear in any SKILL.md — only inside this script.)2.
requireddefaulted toFalse. Even when fields were parsed,valid(len(missing_required) == 0) stayedTruewith missing fields unless each was explicitlyrequired: true— which the emitted schema never sets.Repro (before), with a
fields.yamlin thefields:{cat:[...]}shape/researchproduces:Fix
load_fields_yamlnow acceptsfield_categories: [...],fields: {<cat>: [...]}, flatfields: [...], and a generic fallback that walks for{name: ...}dicts. When no field carries an explicitrequired:key, all fields are treated as required (the script's stated purpose is complete coverage); files that do markrequired:keep their opt-in semantics unchanged.extract_json_fieldsalso descends into JSON keys named after declared categories, so nested output isn't falsely reported as missing.research-en,research-zh,research-codex-en,research-codex-zh.tests/test_validate_json_schemas.py— runs against every copy; asserts both schemas parse and that a JSON missing a defined field now fails.Repro (after):
Note / tradeoff
The "all fields required when none are explicitly marked" rule makes validation stricter than before (previously, unmarked = optional = always pass). This matches what the validator is documented to do, but if you'd prefer keeping
requiredopt-in and gating strictness behind a--strictflag instead, I'm happy to adjust.🤖 Generated with Claude Code