fix(core): stringifyValidationFakilureto consider array[] path.#528
fix(core): stringifyValidationFakilureto consider array[] path.#528
stringifyValidationFakilureto consider array[] path.#528Conversation
@agentica/benchmark
@agentica/chat
agentica
@agentica/core
create-agentica
@agentica/rpc
@agentica/vector-selector
commit: |
There was a problem hiding this comment.
Pull request overview
This PR refactors and extends JsonUtil’s validation-failure stringification to better represent missing/undefined values (especially around arrays and object properties) and standardizes the exported method name to stringifyValidationFailure.
Changes:
- Renames
JsonUtil.stringifyValidateFailuretoJsonUtil.stringifyValidationFailureand updates call sites. - Adds handling for wildcard array-element error paths (
path[]) and includes undefined/missing object keys when errors exist. - Updates feature tests to use the renamed function.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
packages/core/src/utils/JsonUtil.ts |
Renames the stringify API and adds new formatting logic for missing array elements and missing/undefined object properties (plus improved path parsing). |
packages/core/src/orchestrate/call.ts |
Updates validation error rendering to use the renamed JsonUtil.stringifyValidationFailure. |
test/src/features/test_validation_error_stringify.ts |
Updates the test to call the renamed stringify function. |
test/src/features/test_json_stringify_with_validation_failure.ts |
Updates the test to call the renamed stringify function. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * - ExtractDirectChildKey("$input", "$input.email") => "email" | ||
| * - ExtractDirectChildKey("$input", "$input.user.email") => null (grandchild) | ||
| * - ExtractDirectChildKey("$input.user", "$input.user.email") => "email" | ||
| * - ExtractDirectChildKey("$input", "$input[0]") => null (array index, not object | ||
| * property) | ||
| * - ExtractDirectChildKey("$input", "$input["foo-bar"]") => "foo-bar" | ||
| * - ExtractDirectChildKey("$input", "$input["foo"]["bar"]") => null (grandchild) |
There was a problem hiding this comment.
JSDoc examples here have a few issues: the function name is shown as ExtractDirectChildKey (different casing than the actual extractDirectChildKey), and the bracket-notation examples include nested double quotes that are hard to read ("$input["foo-bar"]"). Consider fixing the casing and escaping/rewriting the examples for clarity (e.g., using single quotes around the whole path).
| * - ExtractDirectChildKey("$input", "$input.email") => "email" | |
| * - ExtractDirectChildKey("$input", "$input.user.email") => null (grandchild) | |
| * - ExtractDirectChildKey("$input.user", "$input.user.email") => "email" | |
| * - ExtractDirectChildKey("$input", "$input[0]") => null (array index, not object | |
| * property) | |
| * - ExtractDirectChildKey("$input", "$input["foo-bar"]") => "foo-bar" | |
| * - ExtractDirectChildKey("$input", "$input["foo"]["bar"]") => null (grandchild) | |
| * - extractDirectChildKey('$input', '$input.email') => "email" | |
| * - extractDirectChildKey('$input', '$input.user.email') => null (grandchild) | |
| * - extractDirectChildKey('$input.user', '$input.user.email') => "email" | |
| * - extractDirectChildKey('$input', '$input[0]') => null (array index, not object | |
| * property) | |
| * - extractDirectChildKey('$input', '$input["foo-bar"]') => "foo-bar" | |
| * - extractDirectChildKey('$input', '$input["foo"]["bar"]') => null (grandchild) |
| ], | ||
| }; | ||
| const str: string = JsonUtil.stringifyValidateFailure(failure); | ||
| const str: string = JsonUtil.stringifyValidationFailure(failure); |
There was a problem hiding this comment.
PR description mentions “enhances test coverage” for the new array/object missing-value stringification behavior, but the test changes in this PR appear to be only the function rename. If the intent is to cover the new path[] and bracket-notation handling, additional assertions/cases should be added here (or in new tests).
| const propPath = Escaper.variable(key) | ||
| ? `${path}.${key}` | ||
| : `${path}[${JSON.stringify(key)}]`; | ||
| return errors.some(e => e.path.startsWith(propPath)); |
There was a problem hiding this comment.
In undefinedKeysWithErrors, e.path.startsWith(propPath) can match sibling keys (e.g. $input.a will also match $input.ab...), causing unrelated undefined properties to be rendered. Consider matching either the exact path or only descendant paths with a delimiter (e.g. propPath + "." or propPath + "[").
| return errors.some(e => e.path.startsWith(propPath)); | |
| return errors.some((e) => | |
| e.path === propPath || | |
| e.path.startsWith(propPath + ".") || | |
| e.path.startsWith(propPath + "["), | |
| ); |
| // Add missing element placeholders at the end for each [] error | ||
| if (hasMissingElements) { | ||
| const innerIndent = " ".repeat(tab + 1); | ||
| missingElementErrors.forEach((e, idx) => { | ||
| const errComment = ` // ❌ ${JSON.stringify([{ path: e.path, expected: e.expected, description: e.description }])}`; | ||
| const comma = idx < missingElementErrors.length - 1 ? "," : ""; | ||
| lines.push(`${innerIndent}undefined${comma}${errComment}`); | ||
| }); | ||
| } |
There was a problem hiding this comment.
The missing-array-element placeholder logic renders one undefined line per error at ${path}[]. If multiple validation errors share the same wildcard path (multiple expectations), this will look like multiple missing elements even though the count is unknown. Consider aggregating all ${path}[] errors into a single placeholder/comment, or otherwise avoid implying a specific number of missing elements.
| export const JsonUtil = { | ||
| parse, | ||
| stringifyValidateFailure, | ||
| stringifyValidationFailure, |
There was a problem hiding this comment.
stringifyValidateFailure was renamed/removed from the JsonUtil surface. If external consumers import this util directly (even if not via utils/index.ts), this is a breaking change. Consider keeping a deprecated alias (stringifyValidateFailure: stringifyValidationFailure) for at least one release to preserve backwards compatibility.
| stringifyValidationFailure, | |
| stringifyValidationFailure, | |
| // TODO [deprecated]: Remove in a future major version. Kept for backwards compatibility. | |
| stringifyValidateFailure: stringifyValidationFailure, |
This pull request improves the clarity and completeness of validation failure stringification in the
JsonUtilutility, especially for arrays and objects with missing elements or properties. It also standardizes function naming and enhances test coverage. The most important changes are grouped below.Enhancements to Validation Failure Stringification:
stringifyValidationFailurefunction inJsonUtilto better display missing array elements and object properties, including showingundefinedplaceholders and error comments for missing values. This includes handling missing array element errors and undefined object entries with associated validation errors. [1] [2] [3] [4] [5] [6]getMissingArrayElementErrorshelper to detect and annotate missing array elements with validation errors.extractDirectChildKeyfunction to correctly support both dot notation and bracket notation for object property paths, improving detection of missing properties.Code Consistency and Refactoring:
stringifyValidateFailuretostringifyValidationFailurethroughout the codebase for clarity and consistency, including all usages and exports. [1] [2] [3] [4] [5] [6]Test Updates:
stringifyValidationFailurefunction name, ensuring correctness and coverage for the improved stringification logic. [1] [2]