Conversation
…lations - Extend extractYAMLValueAtPath to support 2-level nested paths (e.g., /permissions/contents) - Fix regex bug in extractTopLevelYAMLValue and extractNestedYAMLValue that allowed cross-line matching - Add extractEnumConstraintPath to find actual sub-path in oneOf error messages - Deduplicate FindClosestMatches results to prevent duplicate suggestions - Add tests for new nested path extraction and enum constraint path functions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…omments Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Improves "Did you mean?" suggestions for enum constraint violations when the schema error is reported at a container path (notably oneOf-driven nested enums like permissions.contents), by extracting the correct nested path and correctly reading the user’s YAML value for fuzzy matching.
Changes:
- Extract the actual enum-constraint JSON path from
oneOferror messages (extractEnumConstraintPath) before looking up the user-entered value. - Fix YAML value extraction to avoid cross-line regex matching and add 2-segment nested path support (
/parent/child). - Deduplicate closest-match results to avoid repeated suggestions and add unit tests for the new helpers.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/parser/schema_suggestions.go | Adds nested-path extraction for enum errors, improves YAML scalar parsing (including 2-level nesting), and deduplicates enum suggestions. |
| pkg/parser/schema_suggestions_test.go | Extends YAML path extraction tests and adds coverage for extracting nested enum constraint paths from oneOf messages. |
Comments suppressed due to low confidence (1)
pkg/parser/schema_suggestions.go:556
extractNestedYAMLValuerecompiles 3 regular expressions (regexp.MustCompile) for every line in the parent block. This is avoidable overhead and makes the loop harder to follow; consider compiling the child regexes once before iterating (or doing a lightweight colon-split parse for the current line) and reusing them.
// Try to match child key with its value (single-quoted, double-quoted, unquoted).
childPrefix := `^\s+` + escapedChild + `[ \t]*:[ \t]*`
reSingle := regexp.MustCompile(childPrefix + `'([^'\n]+)'`)
if match := reSingle.FindStringSubmatch(line); len(match) >= 2 {
return strings.TrimSpace(match[1])
}
reDouble := regexp.MustCompile(childPrefix + `"([^"\n]+)"`)
if match := reDouble.FindStringSubmatch(line); len(match) >= 2 {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| // extractTopLevelYAMLValue extracts the scalar value of a top-level key from raw YAML. | ||
| // Uses horizontal-only whitespace between the colon and value to avoid matching multi-line blocks. | ||
| func extractTopLevelYAMLValue(yamlContent, fieldName string) string { | ||
| escapedField := regexp.QuoteMeta(fieldName) | ||
|
|
||
| // Try single-quoted value: field: 'value' | ||
| reSingle := regexp.MustCompile(`(?m)^\s*` + escapedField + `\s*:\s*'([^'\n]+)'`) | ||
| reSingle := regexp.MustCompile(`(?m)^\s*` + escapedField + `[ \t]*:[ \t]*'([^'\n]+)'`) | ||
| if match := reSingle.FindStringSubmatch(yamlContent); len(match) >= 2 { | ||
| return strings.TrimSpace(match[1]) | ||
| } | ||
| // Try double-quoted value: field: "value" | ||
| reDouble := regexp.MustCompile(`(?m)^\s*` + escapedField + `\s*:\s*"([^"\n]+)"`) | ||
| reDouble := regexp.MustCompile(`(?m)^\s*` + escapedField + `[ \t]*:[ \t]*"([^"\n]+)"`) | ||
| if match := reDouble.FindStringSubmatch(yamlContent); len(match) >= 2 { | ||
| return strings.TrimSpace(match[1]) | ||
| } | ||
| // Try unquoted value: field: value | ||
| reUnquoted := regexp.MustCompile(`(?m)^\s*` + escapedField + `\s*:\s*([^'"\n#][^\n#]*?)(?:\s*#.*)?$`) | ||
| reUnquoted := regexp.MustCompile(`(?m)^\s*` + escapedField + `[ \t]*:[ \t]*([^'"\n#][^\n#]*?)(?:[ \t]*#.*)?$`) | ||
| if match := reUnquoted.FindStringSubmatch(yamlContent); len(match) >= 2 { |
There was a problem hiding this comment.
extractTopLevelYAMLValue is intended to read a top-level scalar, but the regexes are anchored with ^\s*, so they will also match nested/indented occurrences of the same key (e.g. parent:\n engine: copilot would satisfy path /engine). Consider anchoring to the start of line with no indentation (or otherwise explicitly restricting to indent=0) so top-level lookups can’t accidentally read nested keys now that nested path support exists.
See below for a potential fix:
reSingle := regexp.MustCompile(`(?m)^` + escapedField + `[ \t]*:[ \t]*'([^'\n]+)'`)
if match := reSingle.FindStringSubmatch(yamlContent); len(match) >= 2 {
return strings.TrimSpace(match[1])
}
// Try double-quoted value: field: "value"
reDouble := regexp.MustCompile(`(?m)^` + escapedField + `[ \t]*:[ \t]*"([^"\n]+)"`)
if match := reDouble.FindStringSubmatch(yamlContent); len(match) >= 2 {
return strings.TrimSpace(match[1])
}
// Try unquoted value: field: value
reUnquoted := regexp.MustCompile(`(?m)^` + escapedField + `[ \t]*:[ \t]*([^'"\n#][^\n#]*?)(?:[ \t]*#.*)?$`)
| lineIndent := len(line) - len(strings.TrimLeft(line, " \t")) | ||
|
|
||
| // Left parent block if indentation returned to parent level or less | ||
| if lineIndent <= parentIndent { | ||
| break |
There was a problem hiding this comment.
extractNestedYAMLValue will match the child key at any indentation level deeper than the parent because the child regex uses ^\s+... and there’s no check that the key is a direct child of parentKey. This can return the wrong value when the same key appears in a deeper nested mapping within the parent block (grandchild) before the direct child. Consider deriving the direct-child indentation level (e.g., the minimum indent > parentIndent) and only matching keys at that indentation.
This issue also appears on line 549 of the same file.
| // For oneOf errors, the path points to the container (e.g., "/permissions") but | ||
| // the enum constraint is on a nested field (e.g., "/permissions/contents"). | ||
| // Try to extract the actual sub-path from the message. | ||
| actualPath := extractEnumConstraintPath(errorMessage, jsonPath) | ||
| userValue := extractYAMLValueAtPath(frontmatterContent, actualPath) | ||
| if userValue != "" && len(enumValues) > 0 { | ||
| closest := FindClosestMatches(userValue, enumValues, maxClosestMatches) | ||
| closest := sliceutil.Deduplicate(FindClosestMatches(userValue, enumValues, maxClosestMatches)) |
There was a problem hiding this comment.
The PR’s main behavior change is improving enum "Did you mean?" for nested oneOf errors, but the tests currently only cover extractEnumConstraintPath and extractYAMLValueAtPath separately. Add an end-to-end test for generateSchemaBasedSuggestions where jsonPath is the container (e.g. /permissions), the error message contains a nested enum line (e.g. - at '/permissions/contents': value must be one of ...), and frontmatter has a typo (contents: raed), asserting that the returned suggestion includes Did you mean 'read'?.
|
@copilot review comments |
- Anchor extractTopLevelYAMLValue to column 0 (remove ^\s*) to prevent matching indented keys - Track direct-child indent in extractNestedYAMLValue to skip grandchildren with the same key - Add end-to-end test for generateSchemaBasedSuggestions with nested oneOf enum violation - Add regression tests for column-0 anchoring and grandchild-skipping Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed all three review points in commit 1afc55f:
|
extractYAMLValueAtPathto support nested paths (e.g.,/permissions/contents)extractTopLevelYAMLValue/extractNestedYAMLValue(prevented cross-line matching)extractEnumConstraintPathto find actual sub-path inoneOferror messagesFindClosestMatchesresults to prevent duplicate suggestionsextractTopLevelYAMLValueto column 0 (removes^\s*) so indented keys with the same name don't satisfy top-level path lookupsextractNestedYAMLValueto avoid returning grandchild values when a direct child exists at a shallower levelgenerateSchemaBasedSuggestionstest for nestedoneOfenum violation (path=/permissions, typocontents: raed→ "Did you mean 'read'?")✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.