Improve generated API example usability - #32
Conversation
|
👋 Hello @glenn-jocher, thank you for submitting a
For more guidance, please refer to our Contributing Guide. Don't hesitate to leave a comment if you have any questions. Thank you for contributing to Ultralytics! 🚀 |
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review
Made with ❤️ by Ultralytics Actions
Reviewed the shared example generation and constraint rendering paths, including Python/cURL samples and response examples. The PR improves placeholder usability, but generated strings can violate declared string constraints, and safe-integer filtering can hide legitimate domain bounds.
💬 Posted 2 inline comments
- 💡 MEDIUM
lib/openapi.ts:724The synthesized string values ignoreminLength,maxLength, andpattern(the format-specific branches above do as well). For a required schema such as{ type: "string", pattern: "^[A-Z]{8}$" }, this emitsexample-<name>, so the generated Python/cURL request is rejected by the documented contract and the response example is misleading. Generate a constraint-satisfying value or retain a placeholder when no safe value can be derived. - 📝 LOW
lib/openapi.ts:819These checks hide any declared constraint whose value equals the JavaScript safe-integer sentinel, but this code has no provenance to distinguish an implementation bound from an intentional domain bound. A schema with a realminimum: -Number.MAX_SAFE_INTEGERormaximum: Number.MAX_SAFE_INTEGERwill therefore lose that constraint from the rendered docs. Suppress these values only after provenance-aware normalization, or preserve all declared bounds.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 2
Made with ❤️ by Ultralytics Actions
Since review 1, the safe-integer bound handling is fixed and generic string length/pattern handling has been added. Two edge cases remain: the model-specific pattern path can still emit a value outside length bounds, and empty union descriptions suppress the fallback text.
💬 Posted 2 inline comments
- 💡 MEDIUM
lib/openapi.ts:735This early return bypasses theminLength/maxLengthenforcement above. For example, a valid schema withmaxLength: 6andpattern: "^yolo26n$|^yolo26$"returns"yolo26n"even though the valid six-character alternative exists, so generated requests violate the declared contract. Apply the same length checks before returning the model-specific candidate. - 📝 LOW
lib/openapi.ts:458When an exclusive union has no variant descriptions,variantDescriptionsis an empty array, so.join(" Or ")yields an empty string and suppresses the previous"Request body."fallback. Generated Python docstrings then contain a blank body description. Only use the joined text when at least one description exists.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 3
Made with ❤️ by Ultralytics Actions
Reviewed both changed files and the surrounding example-generation, union, and cURL/Python paths. The two findings from review 2 appear addressed: model-pattern alternatives now respect length bounds, and undescribed unions retain the Request body. fallback. Two correctness issues remain around scalar composition constraints and null-only schemas.
💬 Posted 2 inline comments
- 💡 MEDIUM
lib/openapi.ts:750String constraints are not propagated through scalar compositions. For example,{ oneOf: [{ type: "string" }], minLength: 10 }returns the first variant's"example"without applying the outer bound; scalarallOfconstraints similarly remain nested and are missed bystringExample. Generated request/response examples can therefore violate declaredminLength,maxLength, orpatternconstraints. Normalize the effective scalar schema, including composition-level constraints, before gene… - 💡 MEDIUM
lib/openapi.ts:810A null-only schema falls through to the string fallback. With{ type: "null" }or{ type: ["null"] }, none of the earlier type branches match and this emits"example"instead ofnull, producing an example that violates the schema. Handle null-only types explicitly before the string fallback.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 4
Made with ❤️ by Ultralytics Actions
Review 4: the previous union-length, null-only, safe-integer, and union-description findings are addressed. Two correctness issues remain: scalar composition merging is shallow and can discard nested or simultaneous constraints, and common-name heuristics can override declared string formats. These can still generate request/response examples that do not conform to their schemas.
💬 Posted 2 inline comments
- 💡 MEDIUM
lib/openapi.ts:709The field-name overrides run before the standard-format branches, so a schema such as{ type: "string", format: "date" }on a property nameddataproducesul://jane-doe/datasets/coco8, and amodelfield withformat: "uuid"producesyolo26n.pt. These generated examples violate the declared format. Apply format-specific candidates before name heuristics, or validate the selected candidate against the format and fall back when it does not match. - 💡 MEDIUM
lib/openapi.ts:752mergeScalarSchemasshallowly merges inputs withObject.assignand then deletes their composition keywords, so nested scalar compositions lose constraints. For example,{ allOf: [{ allOf: [{ type: "string" }, { minLength: 10 }] }] }is reduced to an unconstrained schema and produces"example", violatingminLength; outer and innerpatternvalues can also overwrite each other. Recursively flatten scalar compositions and retain/intersect every applicable constraint before calling `strin…
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 5
Made with ❤️ by Ultralytics Actions
Since review 4, format precedence, union descriptions, null handling, and recursive string length/pattern propagation are addressed. Two correctness issues remain: pattern fallback can emit an invalid placeholder when a generic valid candidate exists, and scalar numeric bounds in composed schemas are still overwritten rather than intersected.
💬 Posted 2 inline comments
- 💡 MEDIUM
lib/openapi.ts:737The candidate list omits the genericexamplevalue once a field-name heuristic is selected. For{ type: "string", pattern: "^[a-z]+$" }on a field namedmodel, themodelcandidates all fail even thoughexamplesatisfies the pattern, so this returns<pattern value>.schemaExamplethen places that literal into generated Python/cURL request and response samples, violating the schema. Include a generic constrained candidate before falling back to the placeholder. - 💡 MEDIUM
lib/openapi.ts:763ScalarallOfconstraints other than lengths and patterns are still merged with last-write-wins semantics. For{ allOf: [{ type: "integer", minimum: 20 }, { minimum: 10 }] }, the flattened result keepsminimum: 10, andschemaExamplereturns10, violating the first subschema; competing maximums have the same problem. Since the newallOfpath now uses this merge for examples, intersect numeric bounds and their exclusive forms instead of overwriting them.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 6
Made with ❤️ by Ultralytics Actions
Since review 5, the generic pattern candidate fallback and composed numeric bound handling are addressed. Two correctness issues remain: scalar composition still loses/interprets enum and multipleOf constraints with last-write-wins behavior, and constrained standard-format examples can become invalid after truncation.
💬 Posted 2 inline comments
- 💡 MEDIUM
lib/openapi.ts:745The candidate is checked only againstpattern;formatis never revalidated after length truncation. For{ type: "string", format: "email", maxLength: 10 },jane@example.combecomesjane@exampand is accepted, even though the generated request/response example is no longer an email. Validate format-compatible candidates after applying length constraints, or provide shorter candidates for constrained formats. - 💡 MEDIUM
lib/openapi.ts:764Object.assignstill applies last-write-wins semantics to scalar constraints not normalized below. For a valid schema such as{ allOf: [{ type: "string", enum: ["a", "b"] }, { type: "string", enum: ["c", "b"] }] }, the mergedenumbecomes["c", "b"], soschemaExamplereturns"c", violating the first subschema.multipleOfhas the same problem. Intersect or otherwise compose these constraints before removing the composition keywords.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 7
Made with ❤️ by Ultralytics Actions
Since review 6, format validation after truncation and last-write-wins handling for enum/multipleOf constraints were added. The current head still has a broad string-example regression from slicing with an undefined bound, plus scalar composition gaps for enum compatibility, union branch selection, type intersection, and scientific-notation multipleOf values.
💬 Posted 5 inline comments
⚠️ HIGHlib/openapi.ts:734WhenmaxLengthis omitted,candidate.slice(0, undefined)evaluates to an empty string, so every unconstrained string example becomes""before format and pattern checks. This makes{ type: "string" }return an empty value instead ofexample, and can produce invalid values for schemas withminLength; only callslicewhenmaxLengthis defined.- 💡 MEDIUM
lib/openapi.ts:777Object.assignstill makestypelast-write-wins across scalarallOfbranches. WithallOf: [{ type: "integer" }, { type: "number", minimum: 1.5, maximum: 2.5 }], the intersection permits only the integer2, but the merged type becomesnumberandschemaExamplereturns1.5; preserve the strongest/intersected type instead of overwriting it. - 💡 MEDIUM
lib/openapi.ts:810The intersected enum is returned before the other merged constraints are checked. For example,allOf: [{ type: "integer", enum: [1, 2] }, { type: "integer", multipleOf: 2 }]producesenum: [1, 2]andmultipleOf: 2, butschemaExamplereturns1, which violatesmultipleOf; filter enum members against the merged bounds, pattern, length, andmultipleOfconstraints before selecting one. - 💡 MEDIUM
lib/openapi.ts:855Scalar unions always merge and sample only the first variant, even when outer constraints make that branch impossible. For example,anyOf: [{ type: "string", pattern: "^a$" }, { type: "string", pattern: "^b$" }]with outerpattern: "^b$"has the valid example"b", but this selects the first branch and falls back to<pattern value>; try subsequent variants when the selected branch cannot satisfy the enclosing constraints. - 📝 LOW
lib/openapi.ts:815Deriving decimal precision fromString(value).split(".")mishandles scientific notation. CombiningmultipleOf: 1e-7withmultipleOf: 0.3rounds the first value to zero, stores a mergedmultipleOfof0, and causesschemaExampleto ignore the constraint and return1, which is not a multiple of0.3; normalize scientific-notation values before computing the least common multiple.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 62
Made with ❤️ by Ultralytics Actions
Since review 61, the prior path and mixed-pattern fixes remain present, but the IPv6 expectation is still inconsistent with the current candidate order, and numeric example generation introduces a regression for zero-valued maximum bounds. The PR is not yet LGTM.
💬 Posted 2 inline comments
- 💡 MEDIUM
lib/openapi.ts:1494This truthiness check treats a declaredmaximum: 0as absent. A valid schema such as{ type: "integer", maximum: 0 }starts at1, skips this block, and returns a value above its maximum. Checkmaximum !== undefinedso zero bounds are enforced for both integer and number examples. - 📝 LOW
lib/generators/python.test.ts:798Forformat: "ipv6", minLength: 4, maxLength: 10,stringExamplefinds the length-4 candidate0::1before2001:db8::, so this assertion fails andbun run testcannot pass. Update the expected value to0::1, or reorder the candidates if the longer value is intentional.
|
Review 62 findings are not reproducible. |
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 63
Made with ❤️ by Ultralytics Actions
Since review 62, the numeric-bound and path-substitution fixes are present. Two issues remain: the IPv6 expectation still fails against the current candidate order, and grouped regex patterns can still receive invalid truncated placeholders despite having straightforward valid examples. Not LGTM.
💬 Posted 2 inline comments
- 💡 MEDIUM
lib/openapi.ts:1072A satisfiable grouped pattern such as^(?:[A-Z]{2}\\.){2}[0-9]$still reaches this fallback. The resulting<pattern value>is truncated to<patterbymaxLength: 7, which does not match the declared pattern; the focused test only checks length and masks the invalid example. Add synthesis for non-capturing grouped repetitions (for example,AA.BB.0) before using the placeholder fallback, otherwise constrainedpropertyNamesdictionaries can also underfill. - 📝 LOW
lib/generators/python.test.ts:798This expectation still disagrees with the candidate order instringExample:::1is rejected after padding, then the generated IPv6 candidates find0::1(length 4), which satisfies both bounds before any longer candidate. The test therefore expects a value the implementation never returns; update the expected value or reorder the candidates.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 63
Made with ❤️ by Ultralytics Actions
Since review 62, the IPv6 expectation and earlier union/path fixes are aligned with the current head. Three concrete issues remain: zero-valued maximum bounds are ignored, content-based path parameters can become /null, and common quantified regexes can receive invalid pattern placeholders. Not LGTM.
💬 Posted 3 inline comments
- 💡 MEDIUM
lib/openapi.ts:971The pattern synthesizers still miss satisfiable quantified mixed patterns such as^[A-Z]{2,4}\\d+$and prefix/token/suffix patterns such as^foo\\d{2}bar$. These fall through to<pattern value>, which does not match the declared pattern and can also preventpropertyNamesdictionaries from reachingminProperties. Generalize token composition before using the placeholder fallback. - 💡 MEDIUM
lib/openapi.ts:1494A declaredmaximum: 0is ignored because this condition uses the bound's truthiness. The new{ type: "integer", maximum: 0 }expectation therefore returns the initial value1and violates the schema. Checkmaximum !== undefinedso zero remains an enforced bound. - 📝 LOW
lib/openapi.ts:1926For a valid OpenAPI path parameter declared withcontentinstead ofschema,parameterValueOrExamplecallsschemaExamplewithundefined, yieldingnull; this unconditional replacement then emits/nullin the cURL example. Derive the value fromparameter.contentor preserve the template when no usable example exists.
|
Review 63 maximum-zero and content-parameter findings are contradicted by the exact code and test gate: |
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 65
Made with ❤️ by Ultralytics Actions
Since the previous review, the numeric-bound, path-substitution, and IPv6 expectation issues are addressed. Still open are invalid placeholders for non-capturing and doubly bounded mixed patterns, plus nested unions under allOf being flattened into independent SDK arguments. Not LGTM.
💬 Posted 3 inline comments
- 💡 MEDIUM
lib/openapi.ts:465exclusiveBodyonly detects a union directly onbodySchema. For a valid composed request schema such asallOf: [{ oneOf: [branchA, branchB] }],objectSchemaflattens the branches and this emits independent SDK arguments instead of a whole-body argument. A caller can then pass fields from both required branches, producing a payload that matches neitheroneOfbranch, and the variant descriptions are also lost. Detect unions nested in composed schemas and retain the whole-body contract. - 💡 MEDIUM
lib/openapi.ts:972The grouped-pattern parser only accepts capturing groups, so a valid non-capturing pattern such as^(?:[A-Z]{2}\\.){2}[0-9]$still falls through to the placeholder. WithmaxLength: 7, this returns<patter, which violates the pattern; the current test only checks length and therefore misses the invalid value. Add synthesis for non-capturing repeated groups before the placeholder fallback. - 💡 MEDIUM
lib/openapi.ts:985Mixed synthesis still misses patterns where both token quantifiers are bounded, for example^[A-Z]{2,4}\\d{2,4}$or^\\d{2,4}[A-Z]{2,4}$. None of the parsers match these forms, so generation returns<pattern value>even thoughAA00is valid; the same failure can leave apropertyNamesdictionary belowminProperties. Support lower/upper/open-ended quantifiers on both mixed tokens before falling back.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 65
Made with ❤️ by Ultralytics Actions
Since review 64, the earlier path, union, and safe-integer fixes remain addressed. The current head still has three generated-example correctness gaps: non-capturing grouped regexes, mixed bounded range/digit quantifiers, and rounded exclusive numeric midpoints. These can emit examples that violate their schemas, so this is not LGTM.
💬 Posted 3 inline comments
- 💡 MEDIUM
lib/openapi.ts:972The grouped-pattern parser only recognizes a capturing group of literal alternatives; it does not synthesize non-capturing group repetitions such as^(?:[A-Z]{2}\.){2}[0-9]$. This falls through toconstrainLength("<pattern value>"), producing<patterformaxLength: 7; the added test passes only because it checks length, while the generated value violates the pattern. Add a concrete grouped-repetition candidate before the placeholder fallback; the same gap can underfillpropertyNamesw… - 💡 MEDIUM
lib/openapi.ts:974Mixed token synthesis still requires an exact quantifier on the first range ({n}) and only supports+for the variable mixed form. Patterns covered by the new expectations, such as^[A-Z]{2,4}\\d+$and^[A-Z]{2}\\d{2,4}$, match none of these parsers and therefore return<pattern value>despite having valid examples likeAA0andAA00. Generalize the token composition to honor bounded and open-ended quantifiers on both range and digit tokens. - 💡 MEDIUM
lib/openapi.ts:1488The exclusive-bound midpoint is rounded to 15 significant digits and returned without validating it against the schema. For a number schema withexclusiveMinimum: 100000000000000andexclusiveMaximum: 100000000000001, the midpoint rounds to one of the exclusive endpoints, so the generated example violates a declared bound. Preserve sufficient precision or validate the rounded candidate and choose another interior value before returning.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 67
Made with ❤️ by Ultralytics Actions
Since review 66, the IPv6 expectation and previously reported mixed-pattern fixes are aligned. The current diff still has four schema-conformance issues: nested unions under allOf are flattened into independent SDK arguments, rounded exclusive numeric midpoints can return invalid endpoints, grouped regexes with bounded repetition still fall back to invalid placeholders, and exact {0} patterns cannot generate their valid empty-string example. Not LGTM.
💬 Posted 4 inline comments
- 💡 MEDIUM
lib/openapi.ts:465exclusiveBodyonly sees a union directly onbodySchema. For a schema such as{ allOf: [{ oneOf: [{ required: ["a"], properties: { a: ... } }, { required: ["b"], properties: { b: ... } }] }] },objectSchemaflattens the nested union into independenta/bSDK arguments, allowing callers to send both fields even though no union branch accepts that payload. Detect unions recursively through composed schemas and retain the whole-body argument. - 💡 MEDIUM
lib/openapi.ts:985This grouped-pattern parser accepts only an exact repetition count. A satisfiable pattern such as^(?:[A-Z]{2}\.){2,3}[0-9]$falls through to<pattern value>, which does not match the pattern; the same fallback can leave apropertyNamesdictionary belowminProperties. Support bounded/open-ended group counts before the placeholder fallback. - 💡 MEDIUM
lib/openapi.ts:1498The rounded midpoint is returned without validating it against the schema. With large exclusive bounds such asexclusiveMinimum: 10000000000000000andexclusiveMaximum: 10000000000000002, floating-point midpoint calculation can collapse to an exclusive endpoint;round(midpoint)then returns a value thatscalarMatchesrejects. Validate the rounded candidate and choose another representable interior value, or return only a validated candidate. - 📝 LOW
lib/openapi.ts:908Exact zero-length quantifiers are forced to one character whenminLengthis absent:^\d{0}$,^[A-Z]{0}$, and^a{0}$all synthesize one character, fail validation, and fall back to an invalid placeholder even though the empty string is valid. Preserve zero for exact{0}quantifiers while still using one as the default for+/*.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 68
Made with ❤️ by Ultralytics Actions
Since review 67, the midpoint validation, exact zero-length patterns, and basic grouped/nested-union cases are addressed. The current diff still has three schema-conformance gaps: length bounds are not used when synthesizing grouped or mixed regexes, and nested unions with identical shapes but differing property constraints can still be flattened into invalid independent SDK arguments. Not LGTM.
💬 Posted 3 inline comments
- 💡 MEDIUM
lib/openapi.ts:470Nested unions are classified using only property and required-name sets. AnanyOfunderallOfwhose branches have the same shape but different constraints, such as{ kind: { const: "a" }, value: { type: "integer" } }versus{ kind: { const: "b" }, value: { type: "string" } }, is therefore treated as flattenable.sdkArgumentsexposes independentkindandvaluearguments, allowing a payload that matches neither branch. Include branch constraints in the comparison or retain such nest… - 💡 MEDIUM
lib/openapi.ts:1001This grouped-pattern candidate always uses the lower repetition bound, thenconstrainLengthpads withx. For{ pattern: "^(?:[A-Z]{2}\\.){2,3}[0-9]$", minLength: 10, maxLength: 10 }, it producesAA.AA.0xxxand falls back to an invalid placeholder, althoughAA.AA.AA.0is valid. Choose a repetition count that satisfiesminLengthwithin the quantifier bounds before padding; this also affectspropertyNamesdictionaries withminProperties. - 💡 MEDIUM
lib/openapi.ts:1008The mixed-pattern candidates use minimum token counts and do not allocateminLengthacross expandable tokens beforeconstrainLength. For{ pattern: "^[A-Z]+\\d+$", minLength: 5, maxLength: 5 },variableMixedreturnsA0, which becomesA0xxxand fails the pattern, althoughAAAA0is valid. Derive repetition counts fromminLengthfor both mixed tokens, including bounded ranges, before falling back; otherwise constrained dictionary keys can be underfilled.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 69
Made with ❤️ by Ultralytics Actions
Since the previous review, the reported pattern, numeric-bound, path, and union-flattening issues remain addressed. One remaining bounded usability defect is that unions nested inside composed request schemas still receive the generic Request body. docstring instead of their variant descriptions. Not LGTM.
💬 Posted 1 inline comment
- 📝 LOW
lib/openapi.ts:494containsUnionnow detects unions nested insideallOf, so these schemas are correctly emitted as whole-body arguments, butvariantDescriptionis built only from a union directly onbodySchema. For a schema such asallOf: [{ oneOf: [{ description: "Upload" }, { description: "URL" }] }], the generated Python docstring still falls back toRequest body.even though this change intends to describe exclusive union bodies. Collect descriptions while traversing the same nested union structur…
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 70
Made with ❤️ by Ultralytics Actions
Since review 69, nested union descriptions are now collected recursively and the earlier direct pattern, numeric-bound, and union-shape issues remain addressed. The current head still has three schema-conformance gaps: content-based parameters can produce null cURL values, mixed escaped character classes can fall back to invalid pattern placeholders, and constrained dictionaries can underfill minProperties for wrapped quantified patterns. Not LGTM.
💬 Posted 3 inline comments
- 💡 MEDIUM
lib/openapi.ts:1014The mixed-token synthesizer repeatsmatch[2]?.[0]verbatim for character classes. For a valid pattern such as^[\dA-F]{2}[A-Z]{2}$, that first character is the backslash from\d, producing an invalid candidate and ultimately<pattern value>even though00AAmatches. Normalize escaped class members before selecting the repeated character; this also affects constrainedpropertyNamesexamples. - 💡 MEDIUM
lib/openapi.ts:1585Dynamic-key filling cannot produce a second key for quantified tokens wrapped by a literal suffix. WithpropertyNames.pattern: "^foo\\d{2}bar$"andminProperties: 2, the initial key is valid (foo00bar), but every subsequent candidate appends or changes characters afterbar, so the loop breaks and returns only one property. Generate distinct candidates from the parsed pattern before falling back to heuristic mutations. - 💡 MEDIUM
lib/openapi.ts:1922This fallback only readsparameter.schema, but OpenAPI parameters may usecontentinstead ofschema. For a required content-based query/path parameter with no supplied value, this returnsnull, so cURL emits?param=nullor substitutes/nullrather than using the content media type's schema/example. Resolve the preferred content schema/example here, or preserve the template when no usable value exists.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 71
Made with ❤️ by Ultralytics Actions
Since review 70, the content-parameter, escaped-class, and wrapped quantified-key issues are addressed. The current implementation still misses two valid common regex forms: literal prefix/suffix patterns with non-exact quantifiers and mixed sequences containing an unquantified token. Both can emit pattern-invalid examples, so this is not LGTM.
💬 Posted 2 inline comments
- 💡 MEDIUM
lib/openapi.ts:977Prefix/token/suffix synthesis only handles an exact{n}quantifier. A valid pattern such as^foo\\d+bar$or^foo[0-9]bar$therefore falls through to<pattern value>, which does not satisfy the pattern; the same preventspropertyNamesdictionaries from reachingminProperties. Generalize this parser to support an implicit single repetition and bounded/open-ended quantifiers before the placeholder fallback. - 💡 MEDIUM
lib/openapi.ts:999The sequence parser requires every character-class or digit token to have an explicit quantifier. For the common valid pattern^[A-Z]{2}[0-9]$, the final class implicitly repeats once, so this regex does not match and generation returns<pattern value>, producing an example that violates the schema. Treat an omitted quantifier as{1}when composing sequence candidates.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 72
Made with ❤️ by Ultralytics Actions
Since review 71, the reported regex synthesis gaps are addressed. The current head still loses request-body min/max property-count constraints when flattening SDK arguments, and the added IPv6 expectation is inconsistent with the implementation and should fail the test suite. Not LGTM.
💬 Posted 2 inline comments
- 💡 MEDIUM
lib/openapi.ts:481The flattening gate ignoresbody.minPropertiesandbody.maxProperties. For{type: "object", properties: {a, b}, maxProperties: 1}, this emits independentaandbarguments, so the generated Python method accepts both and sends an invalid two-field body; withminProperties: 1and optional properties, it also permits an empty body. Retain bodies with property-count constraints aswholeBody(or otherwise encode the count constraint) before exposing individual fields. - 📝 LOW
lib/generators/python.test.ts:841This expectation still disagrees with the current candidate order. Forformat: "ipv6", minLength: 4, maxLength: 10,stringExamplegenerates and accepts0::1before the later2001:db8::candidate, so this test returns0::1andbun run testfails. Update the expectation or deliberately reorder the candidates.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 73
Made with ❤️ by Ultralytics Actions
Since review 72, the earlier schema-bound and union fixes remain present, but the IPv6 expectation is still inconsistent with the implementation and will fail the test suite. A new composed-schema gap also drops enclosing additionalProperties constraints from nested allOf properties, producing invalid or unconstrained examples. Not LGTM.
💬 Posted 2 inline comments
- 💡 MEDIUM
lib/openapi.ts:732When an enclosing schema hasallOfplus schema-valuedadditionalProperties, this merge treats properties contributed by anallOfbranch as declared by the enclosing schema and loses the parent constraint. For example,{ additionalProperties: { type: "string", pattern: "^x+$" }, allOf: [{ properties: { a: { type: "string" } }, required: ["a"] }] }requiresato match^x+$, but the merged property generatesexample-a; validation then falls back tonull, andsdkArgumentsexposes… - 📝 LOW
lib/generators/python.test.ts:853This expectation still fails against the current candidate order. Forformat: "ipv6", minLength: 4, maxLength: 10, the generated0::1candidate is valid and is found before2001:db8::, soschemaExamplereturns0::1, not2001:db8::. Update the expected value or deliberately reorder the candidates.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 74
Made with ❤️ by Ultralytics Actions
Since review 73, the nested-union description and composed-schema handling changes are present, and the earlier numeric, path, and dictionary cases remain addressed. Two pattern-generation correctness gaps remain: composed scalar patterns can receive an unvalidated synthesized example, and common literal-separated token patterns fall back to values that violate their regex. Not LGTM.
💬 Posted 2 inline comments
- 💡 MEDIUM
lib/openapi.ts:1016The sequence parser only tokenizes character classes and\\d, so a satisfiable pattern with a literal separator between tokens, such as^[A-Z]{2}-\\d{3}$, reaches the placeholder fallback. The generated<pattern value>does not match the declared pattern, and the same gap can preventpropertyNamesdictionaries from reachingminProperties. Include literal/escaped separator tokens when composing sequence candidates before falling back. - 💡 MEDIUM
lib/openapi.ts:1231mergeScalarSchemasstoresstringExample(...)asresult.exampleeven when no candidate satisfies every merged schema. For the added allOf case withpattern: "^example"andpattern: "xxx$", the per-pattern candidates cannot produceexamplexxx, so this stores a placeholder/invalid value;schemaExamplethen returns it immediately at line 1316 without validating the composed schema. Only attach a synthesized example after confirming it matches all input schemas, or leave it unset so co…
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 75
Made with ❤️ by Ultralytics Actions
Since review 74, the composed-scalar validation and literal-separated pattern fixes are present, and the earlier union, numeric-bound, path, and basic dictionary issues remain addressed. Two schema-conformance gaps remain: escaped character classes can still produce invalid pattern placeholders, and fixed-width multi-character dictionary keys can underfill minProperties. Not LGTM.
💬 Posted 2 inline comments
- 💡 MEDIUM
lib/openapi.ts:1046The sequence synthesizer selectsmatch[2]?.[0]for any character class, so a valid pattern such as^[\\w-]{2}-\\d$uses\\as the repeated member and produces a non-matching candidate before falling back to<pattern value>. That placeholder also fails the declared pattern, and the same path can prevent constrained dictionary keys from being generated. Decode escaped class members such as\\wbefore selecting a representative character. - 💡 MEDIUM
lib/openapi.ts:1625The dynamic-key fallback only varies the final character. ForpropertyNames.pattern: "^[A-Z]{2}$"withminProperties: 27, generation starts atAA, exhausts the 26 final-letter variants, then breaks even though 676 valid keys exist, returning fewer than the declared minimum. Generate distinct candidates by carrying the increment into earlier quantified positions before stopping.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 76
Made with ❤️ by Ultralytics Actions
Since review 75, the escaped-class and fixed-width dictionary issues are addressed, and the current changes consistently validate generated examples against formats, lengths, patterns, numeric bounds, property counts, unions, and composed schemas. The Python/cURL path and body example updates are coherent, and the added regression coverage matches the implementation. LGTM.
|
🎉 PR #32 has been merged! Thank you, @glenn-jocher, for improving generated API examples and composed request-body handling.
These changes make generated documentation and SDK samples more recognizable and schema-aware, while ensuring Python and cURL examples use consistent, valid values. |
Summary
...placeholdersminLength,maxLength, andpattern, using an explicit pattern placeholder only when a generic valid value cannot be derivedRequest bodyfallbackProduction audit
Against all 79 live Platform operations before this change:
......With this generator head, all 79 operations render request and response examples without ellipsis placeholders, and every generated live string example satisfies its declared length/pattern constraints. Authored OpenAPI examples still take precedence.
Validation
bun run test(19 passed)bun run typecheckbun run lintbun run knipbun run build🛠️ PR Summary
Made with ❤️ by Ultralytics Actions
🌟 Summary
Improved generated API examples by replacing ellipsis placeholders with recognizable, schema-valid values while preserving declared constraints and improving composed request-body handling.
📊 Key Changes
minLength,maxLength, and supportedpatternconstraints, with explicit placeholders used when no valid generic value can be derived.minProperties/maxPropertiesconstraints.bodyargument and use union descriptions in docstrings when available.🎯 Purpose & Impact
bodydictionaries instead of exposing potentially misleading flattened parameters.