fix(agent): JsonUtil.stringifyValidationFailure with unmappable.#519
fix(agent): JsonUtil.stringifyValidationFailure with unmappable.#519
JsonUtil.stringifyValidationFailure with unmappable.#519Conversation
There was a problem hiding this comment.
Pull request overview
This pull request enhances JSON validation error reporting by tracking which errors can be embedded in the output structure versus those that cannot be mapped, and displays unmappable errors separately. The implementation also centralizes markdown code block formatting within JsonUtil.stringifyValidateFailure.
- Added error tracking mechanism using a
Set<IValidation.IError>to distinguish between mappable and unmappable validation errors - Modified
stringifyValidateFailureto return markdown-formatted output with code blocks, displaying unmappable errors in a separate section - Removed redundant code block markers from
call.tssince the formatting is now handled byJsonUtil
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
test/src/features/test_json_stringify_with_validation_failure.ts |
New test verifying that both regular and unmappable validation errors are correctly included in stringified output |
packages/core/src/utils/stringifyValidateFailure.spec.ts |
Removed comprehensive 1034-line test suite covering various validation scenarios |
packages/core/src/utils/JsonUtil.ts |
Enhanced stringifyValidateFailure to track used errors, wrap output in markdown code blocks, and display unmappable errors separately; threaded usedErrors parameter through stringify and getErrorComment functions |
packages/core/src/orchestrate/call.ts |
Removed redundant markdown code block wrappers around JsonUtil.stringifyValidateFailure calls |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export function test_json_stringify_with_validation_failure(): void { | ||
| const failure: IValidation.IFailure = { | ||
| success: false, | ||
| data: { | ||
| operations: [], | ||
| }, | ||
| errors: [ | ||
| { | ||
| path: "$input.operations", | ||
| expected: "non-empty array", | ||
| value: [], | ||
| }, | ||
| { | ||
| path: "$input.operations", | ||
| expected: "array of objects with 'id' as number", | ||
| value: [], | ||
| }, | ||
| { | ||
| path: "$input.operations[].id", | ||
| expected: "something", | ||
| value: [], | ||
| }, | ||
| ], | ||
| }; | ||
| const str: string = JsonUtil.stringifyValidateFailure(failure); | ||
| TestValidator.predicate("regular")(str.includes( | ||
| JSON.stringify([ | ||
| { | ||
| path: "$input.operations", | ||
| expected: "non-empty array", | ||
| }, | ||
| { | ||
| path: "$input.operations", | ||
| expected: "array of objects with 'id' as number", | ||
| }, | ||
| ]), | ||
| )); | ||
| TestValidator.predicate("unmappable")(str.includes( | ||
| JSON.stringify([{ | ||
| path: "$input.operations[].id", | ||
| expected: "something", | ||
| value: [], | ||
| }], null, 2), | ||
| )); | ||
| } |
There was a problem hiding this comment.
This test file replaces a comprehensive 1034-line test suite that covered missing properties, type mismatches, format violations, nested objects, arrays, complex scenarios, and edge cases. The current test only validates the new unmappable errors feature, resulting in significantly reduced test coverage for the stringifyValidateFailure function. Consider either restoring the deleted tests or creating equivalent test coverage to ensure the function's correctness across all scenarios.
This pull request improves how JSON validation errors are displayed and tracked, especially when some errors cannot be mapped directly into the output structure. It also adds a test to ensure this new behavior works correctly. The main changes are focused on making error reporting more comprehensive and clear.
Enhanced JSON validation error reporting
JsonUtil.stringifyValidateFailureto include all validation errors, and to separately display "unmappable" errors that can't be embedded directly in the JSON output. Used errors are now tracked to distinguish between mapped and unmapped errors.stringifyandgetErrorCommentfunctions inJsonUtil.tsto accept and update ausedErrorsset, ensuring accurate tracking of which errors are shown inline and which are not. [1] [2] [3] [4] [5] [6]Prompt formatting adjustments
call.ts, sinceJsonUtil.stringifyValidateFailurenow handles formatting, preventing duplicate code blocks in output. [1] [2]Testing
test_json_stringify_with_validation_failure.ts) to verify that both regular and unmappable validation errors are correctly included in the stringified output.