Skip to content

Validate yaml file inputs; added tests for inputs#133

Open
LarryOsterman wants to merge 37 commits intomainfrom
larryo/validate_yaml
Open

Validate yaml file inputs; added tests for inputs#133
LarryOsterman wants to merge 37 commits intomainfrom
larryo/validate_yaml

Conversation

@LarryOsterman
Copy link
Member

Accidentally made this PR in a branch of main, not my fork, sorry about that.

Updates the yaml parsers to validate known fields, triggering errors on elements that don't match the expected inputs.

Fixes #132.

Copilot AI review requested due to automatic review settings March 16, 2026 22:45
@github-actions github-actions bot enabled auto-merge (squash) March 16, 2026 22:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens YAML parsing across the codebase to reject unknown fields (addressing Issue #132), and adds/updates tests to ensure invalid YAML inputs fail fast.

Changes:

  • Switch multiple YAML reads from yaml.Unmarshal to yaml.Decoder with KnownFields(true) for stricter decoding.
  • Add tests asserting unknown fields in eval/spec/config YAML cause errors.
  • Update changelog to note the stricter YAML behavior.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
internal/validation/schema.go Uses strict YAML decoding during eval/task schema validation and while extracting tasks.
internal/trigger/spec.go Enforces strict field validation when parsing trigger test specs.
internal/suggest/suggest.go Enforces strict field validation when parsing suggestion YAML and embedded eval YAML.
internal/suggest/suggest_test.go Adds tests for unknown fields in eval YAML (benchmark/config).
internal/projectconfig/config.go Loads .waza.yaml with strict YAML decoding to reject unknown fields.
internal/projectconfig/config_test.go Adds test ensuring .waza.yaml unknown fields return an error.
internal/models/spec.go Loads benchmark spec YAML with strict decoding to reject unknown fields.
internal/models/spec_test.go Adds tests for strict decoding and input deserialization behavior.
internal/models/testcase.go Loads task/testcase YAML with strict decoding and wraps parse errors.
internal/models/baseline_test.go Updates YAML decode in tests to use strict decoder.
internal/jsonrpc/handlers.go Enforces strict YAML decoding for eval.validate RPC endpoint parsing.
internal/generate/generator.go Enforces strict YAML decoding for SKILL.md frontmatter.
cmd/waza/tokens/compare.go Enforces strict YAML decoding when reading .waza.yaml from git refs in --skills mode.
cmd/waza/cmd_check.go Enforces strict YAML decoding when re-reading eval.yaml to count validated tasks.
CHANGELOG.md Notes strict known-field validation in YAML decoders.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 16, 2026 22:51
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses Issue #132 by tightening YAML parsing across the codebase to reject unknown fields (primarily via yaml.Decoder.KnownFields(true)), and adds tests to ensure invalid YAML inputs surface errors.

Changes:

  • Switch multiple YAML parsing sites from yaml.Unmarshal to yaml.NewDecoder(...).KnownFields(true).Decode(...) to reject unknown fields.
  • Add/extend tests to assert unknown YAML fields are rejected (eval YAML, benchmark spec YAML, and .waza.yaml).
  • Update CHANGELOG.md to document stricter YAML field validation.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/validation/schema.go Updates YAML decoding used for schema validation flow.
internal/trigger/spec.go Makes trigger spec parsing strict against unknown YAML fields.
internal/suggest/suggest.go Makes suggestion YAML parsing and embedded eval YAML validation strict.
internal/suggest/suggest_test.go Adds tests asserting unknown fields in eval YAML are rejected.
internal/projectconfig/config.go Makes .waza.yaml parsing strict against unknown fields.
internal/projectconfig/config_test.go Adds test ensuring unknown .waza.yaml fields error.
internal/models/spec.go Makes benchmark spec loading strict against unknown fields.
internal/models/spec_test.go Adds tests covering unknown-field rejection and validation behavior.
internal/models/testcase.go Makes task/testcase YAML loading strict against unknown fields.
internal/models/baseline_test.go Updates baseline tests to decode YAML strictly.
internal/jsonrpc/handlers.go Makes JSON-RPC eval validation strict against unknown fields.
internal/generate/generator.go Makes SKILL.md frontmatter YAML parsing strict (but currently breaks existing SKILL.md files that include extra keys like metadata).
cmd/waza/tokens/compare.go Uses full ProjectConfig when reading .waza.yaml from a git ref.
cmd/waza/cmd_check.go Adds strict YAML decoding when extracting tasks from eval.yaml for readiness counting (currently breaks counting).
CHANGELOG.md Notes strict known-field validation behavior change.
Comments suppressed due to low confidence (1)

cmd/waza/cmd_check.go:1089

  • countValidatedTasks decodes eval.yaml into a struct that only contains tasks, but KnownFields(true) will make decoding fail on every valid eval file that also contains name, config, metrics, etc. This causes the function to always return 0 validated tasks. Consider removing KnownFields(true) here (or decoding into a struct that includes/embeds all expected eval fields) so the task glob list can be extracted from real eval specs.
		return 0
	}
	baseDir := filepath.Dir(report.evalPath)
	count := 0

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 16, 2026 23:08
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens YAML parsing across the codebase to reject unknown fields (addressing issue #132), and adds tests to ensure invalid/unknown YAML keys produce errors.

Changes:

  • Switch multiple YAML parsing paths from yaml.Unmarshal to yaml.Decoder with KnownFields(true) for strict field validation.
  • Add tests covering unknown-field failures for eval YAML and .waza.yaml project config.
  • Update token comparison and validation-related code paths to use updated config/spec parsing.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
internal/validation/schema.go Uses a YAML decoder for schema-based validation parsing.
internal/trigger/spec.go Makes trigger test spec parsing strict with KnownFields(true).
internal/suggest/suggest.go Makes suggestion/eval YAML parsing strict and validates eval YAML.
internal/suggest/suggest_test.go Adds tests asserting unknown eval fields cause errors.
internal/projectconfig/config.go Makes .waza.yaml parsing strict with KnownFields(true).
internal/projectconfig/config_test.go Adds a test asserting unknown .waza.yaml fields error.
internal/models/spec.go Makes LoadBenchmarkSpec strict with KnownFields(true).
internal/models/spec_test.go Adds coverage for unknown fields and invalid specs.
internal/models/testcase.go Makes LoadTestCase strict with KnownFields(true) and improves error context.
internal/models/baseline_test.go Updates YAML decoding in tests to use strict decoder.
internal/jsonrpc/handlers.go Makes eval.validate YAML decoding strict before validation.
internal/generate/generator.go Switches frontmatter parsing to a decoder (non-strict).
cmd/waza/tokens/compare.go Uses ProjectConfig for parsing .waza.yaml when comparing refs.
cmd/waza/cmd_check.go Removes an unused import / minor formatting tweak.
CHANGELOG.md Notes the behavioral change: validating only known YAML fields.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 16, 2026 23:35
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens YAML input validation across the codebase by switching several YAML parsing paths to gopkg.in/yaml.v3 decoders configured to reject unknown fields, and adds tests to ensure unknown fields are surfaced as errors.

Changes:

  • Use yaml.NewDecoder(...).KnownFields(true) for strict YAML parsing in multiple loaders/parsers (specs, test cases, project config, triggers, suggestions).
  • Add tests that verify unknown YAML fields now produce errors for eval YAML and .waza.yaml.
  • Update changelog to note stricter YAML decoding behavior.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/validation/schema.go Switch schema validator YAML parsing to use a decoder (schema validation still drives unknown-field enforcement via JSON schema).
internal/trigger/spec.go Strict trigger spec parsing with KnownFields(true).
internal/suggest/suggest.go Strict parsing for suggestion YAML and eval_yaml payload validation.
internal/suggest/suggest_test.go New tests asserting unknown fields in eval YAML are rejected.
internal/projectconfig/config.go Strict .waza.yaml parsing with KnownFields(true).
internal/projectconfig/config_test.go New test asserting unknown fields in .waza.yaml return an error.
internal/models/testcase.go Strict test case YAML parsing with improved parse error wrapping.
internal/models/spec.go Strict benchmark spec YAML parsing with KnownFields(true).
internal/models/spec_test.go New tests asserting unknown fields (and invalid values) are rejected by spec loading.
internal/models/baseline_test.go Update YAML decoding in tests to use strict decoder.
internal/jsonrpc/handlers.go Strict YAML decode added for eval.validate.
internal/generate/generator.go Frontmatter parsing switched to a decoder (non-strict).
cmd/waza/tokens/compare.go Ref-mode .waza.yaml parsing switched to strict ProjectConfig decoding.
cmd/waza/cmd_check.go Minor import/formatting adjustment around YAML task counting helper.
CHANGELOG.md Note stricter YAML decoding behavior.
Comments suppressed due to low confidence (1)

internal/jsonrpc/handlers.go:229

  • When YAML parsing fails (yerr != nil), the handler still continues with additional semantic checks against the zero-value spec (e.g., appending "no tasks defined" / "executor is required"). This produces misleading errors and can duplicate parse errors already reported via schema validation. Consider returning early on parse failure, or gating the subsequent checks on yerr == nil (including the Tasks/EngineType checks).
	yerr := decoder.Decode(&spec)
	if yerr != nil {
		errs = append(errs, fmt.Sprintf("parse error: %v", yerr))
	}

	// Schema validation via validation package
	schemaEvalErrs, schemaTaskErrs, _ := validation.ValidateEvalFile(p.Path)
	for _, e := range schemaEvalErrs {
		errs = append(errs, fmt.Sprintf("schema: %s", e))
	}
	for file, fileErrs := range schemaTaskErrs {
		for _, e := range fileErrs {
			errs = append(errs, fmt.Sprintf("%s: %s", file, e))
		}
	}

	if yerr == nil {
		if verr := spec.Validate(); verr != nil {
			errs = append(errs, verr.Error())
		}
	}

	if len(spec.Tasks) == 0 {
		errs = append(errs, "no tasks defined")
	}

	if spec.Config.EngineType == "" {
		errs = append(errs, "executor is required")
	}

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 16, 2026 23:44
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses #132 by tightening YAML parsing across core WAZA config/spec loaders so unknown fields are rejected (primarily via yaml.v3 Decoder with KnownFields(true)), and adds tests to ensure unknown-field inputs fail validation.

Changes:

  • Switched several YAML parsing sites from yaml.Unmarshal to yaml.NewDecoder(...).KnownFields(true).Decode(...) for stricter field validation.
  • Added/updated tests to assert unknown YAML fields are rejected in eval/spec and .waza.yaml config parsing.
  • Updated changelog to document the behavior change.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/validation/schema.go Uses yaml.Decoder for minimal task glob extraction + schema validation parsing path.
internal/trigger/spec.go Trigger test spec parsing now rejects unknown fields (KnownFields(true)).
internal/suggest/suggest.go Suggestion parsing and embedded eval YAML validation updated to use strict decoding in several places.
internal/suggest/suggest_test.go Adds tests asserting unknown fields in generated eval YAML are rejected.
internal/projectconfig/config.go .waza.yaml parsing now rejects unknown fields (KnownFields(true)).
internal/projectconfig/config_test.go Adds test ensuring .waza.yaml unknown fields produce an error.
internal/models/spec.go Benchmark spec loader now rejects unknown fields (KnownFields(true)).
internal/models/spec_test.go Adds test coverage for unknown fields / invalid values during spec loading.
internal/models/testcase.go Test case loader now rejects unknown fields and wraps parse errors.
internal/models/baseline_test.go Updates YAML decoding in baseline tests to use strict decoder.
internal/jsonrpc/handlers.go eval.validate YAML parsing now uses strict decoder.
internal/generate/generator.go Frontmatter YAML parsing switched to yaml.Decoder.
cmd/waza/tokens/compare.go Attempts strict decode of .waza.yaml at a ref, with permissive fallback for paths.skills.
cmd/waza/cmd_check.go Minor formatting-only change (blank line).
CHANGELOG.md Notes new “validate only known fields in YAML decoders” behavior.
Comments suppressed due to low confidence (2)

internal/jsonrpc/handlers.go:229

  • In eval.validate, when YAML parsing fails (yerr != nil) the handler continues to run additional checks against the zero-value spec (e.g., len(spec.Tasks)==0, missing executor). This can add misleading errors that are artifacts of the failed parse rather than real validation results. Consider returning early on parse errors, or gating all spec-derived checks (tasks/executor/spec.Validate) behind yerr == nil so the error list reflects the actual failure.
	yerr := decoder.Decode(&spec)
	if yerr != nil {
		errs = append(errs, fmt.Sprintf("parse error: %v", yerr))
	}

	// Schema validation via validation package
	schemaEvalErrs, schemaTaskErrs, _ := validation.ValidateEvalFile(p.Path)
	for _, e := range schemaEvalErrs {
		errs = append(errs, fmt.Sprintf("schema: %s", e))
	}
	for file, fileErrs := range schemaTaskErrs {
		for _, e := range fileErrs {
			errs = append(errs, fmt.Sprintf("%s: %s", file, e))
		}
	}

	if yerr == nil {
		if verr := spec.Validate(); verr != nil {
			errs = append(errs, verr.Error())
		}
	}

	if len(spec.Tasks) == 0 {
		errs = append(errs, "no tasks defined")
	}

	if spec.Config.EngineType == "" {
		errs = append(errs, "executor is required")
	}

internal/suggest/suggest.go:212

  • ParseResponse enables KnownFields(true) when decoding the outer Suggestion wrapper. This makes the suggestion parser brittle: any extra keys in the model output (e.g., "notes", "reason") will cause decode to fail even if eval_yaml and generated files are present, and the later fallback (validateEvalYAML(normalized)) won’t apply to the wrapper format. Consider keeping strict parsing only for the embedded eval_yaml (via validateEvalYAML) and decoding the outer Suggestion permissively (or trying permissive decode if strict fails).
	var s Suggestion
	decoder := yaml.NewDecoder(strings.NewReader(normalized))
	decoder.KnownFields(true)
	if err := decoder.Decode(&s); err == nil && strings.TrimSpace(s.EvalYAML) != "" {
		if err := validateEvalYAML(s.EvalYAML); err != nil {
			return nil, err
		}
		return &s, nil
	}

Copy link
Contributor

Copilot AI commented Mar 17, 2026

@LarryOsterman I've opened a new pull request, #134, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI review requested due to automatic review settings March 19, 2026 22:09
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

site/src/content/docs/guides/eval-yaml.mdx:1

  • This example uses pattern, but elsewhere in this PR the text grader schema/docs use regex_match (and related keys) rather than pattern. Please align this example with the actual supported config fields (or document pattern explicitly if it’s still supported), otherwise strict parsing/schema validation will cause copy-pasted examples to fail.
    site/src/content/docs/guides/graders.mdx:1
  • Minor wording issue: 'the triggers grader documentation' reads as incorrect/awkward. Consider changing to 'the trigger grader documentation' (or 'Trigger grader documentation') for clarity.

Copilot AI review requested due to automatic review settings March 19, 2026 22:30
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated 4 comments.

Copy link
Contributor

Copilot AI commented Mar 19, 2026

@LarryOsterman I've opened a new pull request, #150, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI review requested due to automatic review settings March 19, 2026 23:07
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated 4 comments.

Copy link
Contributor

Copilot AI commented Mar 19, 2026

@LarryOsterman I've opened a new pull request, #151, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI review requested due to automatic review settings March 20, 2026 18:51
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 35 out of 35 changed files in this pull request and generated 4 comments.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comments suppressed due to low confidence (1)

schemas/task.schema.json:108

  • resourceRef currently allows an empty object (neither path nor content required). In the runtime, resources with an empty path are silently skipped when writing to the workspace, which can make mistakes in task YAML hard to diagnose. Consider tightening the schema to require at least one of path/content, and (if content is present) require a non-empty path so inline resources can’t be ignored accidentally.
    "resourceRef": {
      "type": "object",
      "additionalProperties": false,
      "description": "A file reference by path or inline content.",
      "properties": {
        "path": {
          "type": "string",
          "description": "Path to the file relative to context_dir."
        },
        "content": {
          "type": "string",
          "description": "Inline file content."
        }
      }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WAZA does not validate input yaml fields.

3 participants