Skip to content

Sanitize paths in produced json - #85

Closed
JanKrivanek wants to merge 2 commits into
dotnet:mainfrom
JanKrivanek:dev/jankrivanek/sanitize-paths
Closed

Sanitize paths in produced json#85
JanKrivanek wants to merge 2 commits into
dotnet:mainfrom
JanKrivanek:dev/jankrivanek/sanitize-paths

Conversation

@JanKrivanek

Copy link
Copy Markdown
Member

Context

LLM Judge can be adding paths to result json that cause it to be unparseable - e.g.:

https://github.com/dotnet/skills/actions/runs/22298025045/job/64498823613#step:6:61

So sanitizing paths to prevent results being flagged as wrong

Copilot AI review requested due to automatic review settings February 23, 2026 11:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a small JSON sanitization step in the skill-validator’s LLM judge parsing path to prevent invalid escape sequences (commonly from Windows-style paths) from breaking JSON.parse, and introduces unit tests to cover these cases.

Changes:

  • Add sanitizeJsonEscapes() utility to repair invalid JSON escape sequences inside JSON strings.
  • Apply sanitization before JSON.parse in both the independent judge and pairwise judge parsers.
  • Add Vitest coverage for common valid/invalid escape scenarios and ignore generated validator results output.

Reviewed changes

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

Show a summary per file
File Description
eng/skill-validator/src/json-utils.ts Introduces sanitizeJsonEscapes() to rewrite invalid escapes to valid JSON escapes.
eng/skill-validator/src/judge.ts Sanitizes extracted JSON before parsing and reports sanitized JSON on parse errors.
eng/skill-validator/src/pairwise-judge.ts Sanitizes extracted JSON before parsing in pairwise judge flow.
eng/skill-validator/tests/json-utils.test.ts Adds unit tests covering valid escapes, invalid path escapes, and multiline judge output.
.gitignore Ignores skill-validator’s default .skill-validator-results/ output directory.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread eng/skill-validator/src/json-utils.ts Outdated
Comment thread eng/skill-validator/src/json-utils.ts
Comment thread eng/skill-validator/src/pairwise-judge.ts Outdated
Comment thread eng/skill-validator/tests/json-utils.test.ts Outdated
@JanKrivanek

Copy link
Copy Markdown
Member Author

Fixed by other PRs

moesac0970 pushed a commit to moesac0970/skills that referenced this pull request Jul 4, 2026
…xed: removed dangling writing-mstest-tests ref
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.

2 participants