[Ruby] Add new ruby-idiomatic client generator (opt-in idiomatic redesign)#24220
Open
n-rodriguez wants to merge 1 commit into
Open
[Ruby] Add new ruby-idiomatic client generator (opt-in idiomatic redesign)#24220n-rodriguez wants to merge 1 commit into
ruby-idiomatic client generator (opt-in idiomatic redesign)#24220n-rodriguez wants to merge 1 commit into
Conversation
Member
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
40 issues found across 792 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/metrics.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/metrics.rb:12">
P1: Passing an uppercase symbol `:GET` to `@connection.call` will raise `ArgumentError: unknown http method: :GET` at runtime. `Connection#call` forwards the method directly to Faraday's `run_request`, which only accepts lowercase symbols (`:get`, `:post`, etc.). Consider normalizing the method in `Connection#call` (e.g., `method.to_s.downcase.to_sym`) or updating the generator template to emit lowercase symbols.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/geo_line_string.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/geo_line_string.rb:20">
P2: The model initializer silently ignores unknown keyword arguments and does not enforce required attributes, which can mask caller typos and allow invalid instances to be created. The `initialize` method filters attributes with `respond_to?("#{k}=")`, so a typo like `point:` instead of `points:` is silently dropped. Although `points` is marked `required: true`, the constructor never validates presence or calls the `valid?` method provided by `Qdrant::Validations`. This defers errors to the server or hides them entirely, making debugging difficult for consumers of the generated client. Consider raising `ArgumentError` for unknown keys and invoking required-field validation during initialization or serialization.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/geo_radius.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/geo_radius.rb:25">
P2: The model `initialize` silently ignores unknown keyword arguments, which hides caller typos and makes debugging harder. For example, passing `raius:` instead of `radius:` leaves `radius` as `nil` with no error, only failing later during validation or serialization.
The same codebase already provides a stricter `build` method in the `Validations` mixin that raises `NoMethodError` for unknown keys. Consider making `initialize` consistent by removing the `respond_to?` guard (or explicitly raising `ArgumentError` for unknown attributes) so typos fail fast at construction time.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/ruby-idiomatic/client.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/ruby-idiomatic/client.mustache:13">
P2: The `Client` template emits namespace accessors as raw `def {{name}}` methods without guarding against collisions with the class's own API (`initialize`, `configuration`, `connection`) or inherited Ruby methods. If a generated namespace name matches one of these (e.g. `connection`), `client.connection` will silently return a sub-client instead of the `Connection` instance, breaking any code that relies on the transport object. Consider maintaining a reserved-method list for namespace accessor names (similar to the existing `RESERVED_MODEL_NAMES` guard for models) and renaming or prefixing colliding accessors in the codegen.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/create_alias.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/create_alias.rb:25">
P2: The model initializer silently ignores unknown keys and does not enforce required fields, allowing invalid objects to be created without early failure. The `initialize` method filters kwargs with `respond_to?("#{k}=")`, so typos (e.g., `aliasname:`) are silently dropped. It also does not validate that `required: true` fields like `collection_name` and `alias_name` are present. Unless `Qdrant::Validations` is automatically run on every instance before serialization, callers will only discover the error much later—usually when the API rejects the request. Consider either raising on unknown keys or automatically invoking presence/validations in `initialize`.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/init_from.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/init_from.rb:20">
P2: The model declares `collection` with `required: true`, but the constructor does not enforce it, so an invalid object can be created silently and only fail later when `valid?` is called. Consider triggering validation inside the constructor (or `build`/`from_hash`) so `required` is actually enforced at creation time.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/root.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/root.rb:13">
P2: This API endpoint uses an absolute path `'/'`, which Faraday resolves by replacing the entire path component of the configured `base_url`. If the server URL includes a non-root base path (e.g. `https://host/api`), this request is sent to `https://host/` instead of `https://host/api/`. Consider making paths relative in `Connection#call` (e.g. by stripping a leading `/` before passing to `run_request`) or using a relative path here so the base URL path prefix is preserved.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/discover_request_batch.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/discover_request_batch.rb:20">
P2: `searches` is declared as `required: true`, but the `initialize` method only assigns whatever keyword arguments are provided without enforcing that required attributes are present. Consider validating required fields during construction (or invoking validation from `initialize`) so that missing required attributes fail immediately with a clear error rather than surfacing later during serialization or request execution.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/cluster_status.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/cluster_status.rb:19">
P2: The `oneOf` wrapper uses a first-match-wins strategy that violates OpenAPI `oneOf` semantics. The `build` method returns the first candidate that successfully casts without verifying that no other candidates also match. If a payload is ambiguous and satisfies multiple schemas, the code silently deserializes to whichever candidate is listed first, which can mask data-modeling bugs and propagate incorrect type interpretation. For proper `oneOf` compliance, the wrapper should collect all matching candidates and raise an error (or otherwise signal ambiguity) when more than one matches.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/app_features_telemetry.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/app_features_telemetry.rb:35">
P2: The `initialize` method silently ignores unknown keyword arguments because of the `respond_to?("#{k}=")` guard, so a typo like `AppFeaturesTelemetry.new(debg: true)` passes without error and simply leaves `debug` as `nil`. For a model where every field is declared `required: true`, this is a significant debuggability issue. Consider raising `ArgumentError` for unknown keys, and validating required fields before the object is considered fully initialized.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/create_alias_operation.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/create_alias_operation.rb:20">
P2: The model declares `create_alias` as `required: true`, but the constructor does not enforce its presence, so the object can be instantiated in an invalid state and the nil value is serialized into API requests. Consider invoking validation in `initialize` (e.g., calling `valid?` or raising if required attributes are missing) so that `required: true` is enforced at the point of object construction.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/ruby-idiomatic/configuration.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/ruby-idiomatic/configuration.mustache:13">
P2: The `Configuration` initializer silently drops unknown options due to the `if respond_to?("#{k}=")` guard. Misspelled keys (e.g., `baseurl:`, `acess_token:`) are ignored without feedback, so users get defaults instead of their intended values and may face hard-to-diagnose runtime failures. Consider removing the guard so unrecognized options raise immediately, or explicitly raise an `ArgumentError` with the unknown key name.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/count_request.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/count_request.rb:31">
P2: The initializer silently discards unknown keyword arguments because of the `if respond_to?` guard. In a request model like `CountRequest`, this means a simple caller typo (e.g., `excat: false`) gets dropped without error, and the `@exact = true` default then accidentally applies—producing an unintended API payload with no warning. A fail-fast approach would surface these mistakes immediately during model construction rather than allowing a subtly incorrect request to reach the API.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/ruby-idiomatic/api.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/ruby-idiomatic/api.mustache:14">
P2: Required OpenAPI parameters are not guarded against explicit `nil` values at runtime. Ruby keyword arguments without defaults prevent omission, but passing `nil` explicitly is still allowed and silently produces invalid requests: path params become empty string segments (`nil.to_s`), query/header/form entries are dropped by `Connection#encode_query`/`merge_headers`/`wrap_form` via `.compact`, and body params pass through as `nil`. This violates the OpenAPI required contract and leads to cryptic server errors. Consider adding runtime `nil` checks (e.g., `raise ArgumentError` or a validation helper) for required params before calling `@connection.call`.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/ruby-idiomatic/connection.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/ruby-idiomatic/connection.mustache:77">
P1: The `deserialize` method unconditionally parses String bodies as JSON without rescuing `JSON::ParserError`, which means successful responses containing non-JSON payloads (e.g., plain text, whitespace-only strings, or HTML from a misbehaving server) will raise an unhandled exception that bypasses the `ApiError` wrapping in `call` — breaking the single-choke-point error contract. Consider rescuing `JSON::ParserError` in `deserialize` (or `call`) and re-raising it as an `ApiError` so consumers only need to rescue one exception type.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/cluster.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/cluster.rb:14">
P2: The `recover` method deserializes the `POST /cluster/recover` response into `Qdrant::Models::CreateShardKey200Response`, which is named for a different operation (shard-key creation) and looks like a response-type mapping error. If the actual cluster-recovery schema differs, callers could hit deserialization failures or silently drop fields. Please verify the OpenAPI response schema for this operation and update the bound model accordingly.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/collection_info.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/collection_info.rb:60">
P2: The constructor silently drops unknown keys, which removes normal Ruby keyword-argument safety. A typo or schema drift leaves required attributes nil without any immediate feedback, and since `Qdrant::Validations` is only checked manually (via `valid?`), the invalid object can be serialized and used unless the caller remembers to validate. Consider raising `ArgumentError` for unknown attributes so typos fail fast.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/binary_quantization.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/binary_quantization.rb:20">
P2: The `BinaryQuantization` model marks `binary` as `required: true`, but `initialize` does not enforce that required attributes are present or non-nil. This allows invalid instances to be created silently (for example, `BinaryQuantization.new` or `BinaryQuantization.new(binary: nil)` succeeds even though `binary` is required), and validation is only discoverable later through an explicit `valid?` check. Consider enforcing required fields during initialization—either by validating after assignment in `initialize` or by adding a `validate!` mechanism that runs automatically—so that `required: true` in the DSL actually guarantees a valid instance at construction time.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/discover_request.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/discover_request.rb:73">
P2: The generated model initializer silently ignores unknown keyword arguments, which can mask caller typos or schema drift. Since `Validations.build` already raises `NoMethodError` for unknown keys and `from_hash` handles deserialization strictly, `initialize` should be strict too rather than silently dropping mistyped attributes like `limt:`. Removing the `respond_to?` guard makes construction fail fast and consistent with the rest of the DSL.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/delete_alias.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/delete_alias.rb:20">
P2: A required field (`alias_name`) is declared but never enforced during object construction. Since `initialize` simply assigns whatever keys are provided, callers can create an invalid `DeleteAlias` instance (e.g., via a typo or by omitting the argument entirely) without any immediate error. The invalid state then propagates through serialization (`to_hash` includes `nil` for required fields) and only surfaces as a server-side failure later. Consider validating required fields inside `initialize` or at least invoking the existing `list_invalid_properties` logic so the client fails fast.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/match.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/match.rb:27">
P2: The `anyOf` wrapper silently returns `nil` when no candidate type matches the input, which can hide API payload drift or unexpected data shapes. Consider whether the generator (or this specific wrapper) should raise a descriptive deserialization error instead of defaulting to `nil`, so callers fail fast rather than encountering deferred `NoMethodError`s downstream.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/integer_index_params.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/integer_index_params.rb:40">
P2: The model marks `type` as `required: true`, but `initialize` never validates that required attributes are present. An instance created without `type` will only fail later—often as a cryptic API error—instead of failing fast at construction time. After assigning attributes, call `valid?` (or `list_invalid_properties`) and raise an `ArgumentError` so missing required fields are caught immediately.</violation>
<violation number="2" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/integer_index_params.rb:40">
P2: The `initialize` method silently ignores any unknown attribute keys (`if respond_to?("#{k}=")`), which means typos or newly introduced fields are dropped without feedback. In an API client this can mask bugs and cause silent data loss (e.g., a caller passes `on_diskk:` but the misspelled attribute is ignored and the default is sent instead). Consider raising on unknown keys or capturing them in an `additional_properties` hash so callers get clear feedback when they pass invalid attributes.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/configuration.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/configuration.rb:13">
P2: Unknown configuration keys are silently dropped, which makes typos (e.g., `timeuot:`, `accessTokn:`) fail open with no feedback and leaves defaults active. This can cause hard-to-diagnose misconfiguration bugs at runtime. Consider either failing fast on unknown keys or warning the caller when an option is unrecognized.</violation>
<violation number="2" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/configuration.rb:51">
P2: Empty-string credentials are treated as present and produce malformed auth headers instead of being omitted. In Ruby, empty strings are truthy, so `api_key: ''` emits `api-key: ''` and `access_token: ''` emits `Authorization: Bearer `. This can cause server-side auth failures when headers are present but empty. Consider checking non-emptiness explicitly, e.g. `if api_key && !api_key.empty?` and `if access_token && !access_token.empty?`.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/is_empty_condition.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/is_empty_condition.rb:20">
P2: The model constructor does not enforce `required: true` constraints, and serialization proceeds without validation, so instances missing required fields can still be sent to the API as invalid payloads. Consider validating required attributes before serialization or at least during initialization so that missing required fields fail fast on the client side rather than producing a server error.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/field_condition.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/field_condition.rb:51">
P2: The `initialize` method silently ignores unknown keys and does not enforce required fields at construction time. Because it only assigns values when `respond_to?("#{k}=")` is true, a typo like `:kee` instead of `:key` is dropped without error. Similarly, the required `key` attribute is not validated during initialization, so `FieldCondition.new` can create an instance missing mandatory data. This can mask caller mistakes and lead to invalid request payloads. Consider raising `ArgumentError` for unrecognized keys and validating required attributes inside `initialize`, or at least documenting that validation must be triggered explicitly.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/collections_telemetry.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/collections_telemetry.rb:26">
P2: `number_of_collections` is declared `required: true`, but `initialize` performs only best-effort setter assignment and never enforces requiredness. The `Validations` mixin provides `valid?` (which checks the `required` rule), yet `initialize`, `to_hash`, and `Connection#call` never invoke it, so an instance missing a required field can be serialized and sent to the API without any client-side pushback. Consider wiring validation into the construction or serialization path (e.g., calling `valid?` after attribute assignment in `initialize` or before serialization in the connection layer) so required fields are actually enforced.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/context_query.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/context_query.rb:21">
P2: The `initialize` method silently ignores unknown keyword arguments, which masks caller typos and can produce subtly invalid objects (especially for optional fields). The `Validations` module’s `build` method raises on unknown keys, but `initialize` does not, so there is no consistent fail-fast behavior. Consider tightening the generator template (`partial_model_generic.mustache`) to raise on unknown keys—for example by replacing the conditional with an explicit `ArgumentError`—so that typos fail immediately instead of being swallowed.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/collections.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/collections.rb:38">
P2: The `Collections` class exposes duplicate endpoint methods that perform identical HTTP calls with no discernible difference: `cluster` and `cluster_get` are both GET `/collections/{collection_name}/cluster`, and `cluster_post` and `cluster_post_1` are both POST to the same path with identical arguments, query, and body handling. This redundant generated surface creates ambiguity for consumers about which method to call and increases maintenance cost. Please confirm whether these are intentional aliases driven by operationId routing, or if the generator is inadvertently emitting duplicates due to naming collision handling. If unintended, the generator logic should deduplicate equivalent operations so only one method is emitted per unique HTTP verb + path combination.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/collection_description.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/collection_description.rb:20">
P2: The model declares `name` as `required: true`, but `initialize` does not enforce that required fields are present. This allows invalid instances to be created silently; downstream serialization or request building can fail with nil errors far from the origin. Consider validating required attributes inside `initialize` (or via a post-init hook) so that `required: true` is actually enforced at object construction time rather than only when `valid?` is called explicitly.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/ruby-idiomatic/validations.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/ruby-idiomatic/validations.mustache:79">
P2: The `validate_attribute` method performs numeric comparisons (`value > rules[:maximum]` and `value < rules[:minimum]`) without first checking that `value` is a comparable numeric type. Because `coerce` in `from_hash` can return raw untyped data and setters are plain `attr_accessor`, a String or other non-numeric assigned to a numeric field causes `valid?` to raise `ArgumentError` instead of returning false. Similarly, `max_items`/`min_items` call `value.length` without confirming the value is a collection, which can raise `NoMethodError`. Consider guarding these checks with type/respond-to guards so that `valid?` remains a safe, exception-free query.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant.rb:28">
P1: The `qdrant.rb` loader setup is missing the `collapse` call that its own comment says is required when the `apiNamespace` wrapper is disabled. Because the generated files under `lib/qdrant/api/` do not define a `Qdrant::Api` module, Zeitwerk will infer the wrong constant paths and autoloading will break. Consider adding `@loader.collapse(...)` before `@loader.setup` (or generating it conditionally in the template) so the directory layout matches the actual module nesting.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/ruby-idiomatic/gem.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/ruby-idiomatic/gem.mustache:18">
P1: The template uses `Zeitwerk::Loader.for_gem`, which infers the gem's root namespace from the entry file's basename (`{{gemName}}.rb`), while the file simultaneously declares `module {{moduleName}}`. The codegen allows both values to be set independently via CLI options with no validation that they produce the same Zeitwerk root constant. If they differ—for example, when a user sets `--module-name OpenAPIClient` (which produces `gemName` = `open_api_client` and a Zeitwerk-expected root of `OpenApiClient`)—autoloading will fail because Zeitwerk expects one namespace while the file defines another. Consider either enforcing alignment between `gemName` and `moduleName` in `processOpts()` or configuring Zeitwerk with an explicit inflector/namespace binding so the loader knows the actual root constant.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/discover_input_context.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/discover_input_context.rb:18">
P2: The anyOf `build` loop assumes `Qdrant::Polymorphism.cast` returns `nil` on mismatch, but if a non-matching candidate raises an exception (e.g., `NoMethodError` when a model's `from_hash` receives an Array instead of a Hash), the loop aborts and later candidates are never tried. This breaks the intended "first candidate that validates" fallback behavior. Consider rescuing exceptions during candidate evaluation so mismatching candidates gracefully return `nil` instead of crashing the entire anyOf resolution.</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 200 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
Re-trigger cubic
|
|
||
| def list(anonymize: nil) | ||
| @connection.call( | ||
| :GET, |
Contributor
There was a problem hiding this comment.
P1: Passing an uppercase symbol :GET to @connection.call will raise ArgumentError: unknown http method: :GET at runtime. Connection#call forwards the method directly to Faraday's run_request, which only accepts lowercase symbols (:get, :post, etc.). Consider normalizing the method in Connection#call (e.g., method.to_s.downcase.to_sym) or updating the generator template to emit lowercase symbols.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/metrics.rb, line 12:
<comment>Passing an uppercase symbol `:GET` to `@connection.call` will raise `ArgumentError: unknown http method: :GET` at runtime. `Connection#call` forwards the method directly to Faraday's `run_request`, which only accepts lowercase symbols (`:get`, `:post`, etc.). Consider normalizing the method in `Connection#call` (e.g., `method.to_s.downcase.to_sym`) or updating the generator template to emit lowercase symbols.</comment>
<file context>
@@ -0,0 +1,20 @@
+
+ def list(anonymize: nil)
+ @connection.call(
+ :GET,
+ '/metrics',
+ type: nil,
</file context>
| json_key: 'collections', | ||
| required: false | ||
|
|
||
| def initialize(**attrs) |
Contributor
There was a problem hiding this comment.
P2: number_of_collections is declared required: true, but initialize performs only best-effort setter assignment and never enforces requiredness. The Validations mixin provides valid? (which checks the required rule), yet initialize, to_hash, and Connection#call never invoke it, so an instance missing a required field can be serialized and sent to the API without any client-side pushback. Consider wiring validation into the construction or serialization path (e.g., calling valid? after attribute assignment in initialize or before serialization in the connection layer) so required fields are actually enforced.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/collections_telemetry.rb, line 26:
<comment>`number_of_collections` is declared `required: true`, but `initialize` performs only best-effort setter assignment and never enforces requiredness. The `Validations` mixin provides `valid?` (which checks the `required` rule), yet `initialize`, `to_hash`, and `Connection#call` never invoke it, so an instance missing a required field can be serialized and sent to the API without any client-side pushback. Consider wiring validation into the construction or serialization path (e.g., calling `valid?` after attribute assignment in `initialize` or before serialization in the connection layer) so required fields are actually enforced.</comment>
<file context>
@@ -0,0 +1,31 @@
+ json_key: 'collections',
+ required: false
+
+ def initialize(**attrs)
+ attrs.each { |k, v| public_send("#{k}=", v) if respond_to?("#{k}=") }
+ end
</file context>
| def list | ||
| @connection.call( | ||
| :GET, | ||
| '/', |
Contributor
There was a problem hiding this comment.
P2: This API endpoint uses an absolute path '/', which Faraday resolves by replacing the entire path component of the configured base_url. If the server URL includes a non-root base path (e.g. https://host/api), this request is sent to https://host/ instead of https://host/api/. Consider making paths relative in Connection#call (e.g. by stripping a leading / before passing to run_request) or using a relative path here so the base URL path prefix is preserved.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/root.rb, line 13:
<comment>This API endpoint uses an absolute path `'/'`, which Faraday resolves by replacing the entire path component of the configured `base_url`. If the server URL includes a non-root base path (e.g. `https://host/api`), this request is sent to `https://host/` instead of `https://host/api/`. Consider making paths relative in `Connection#call` (e.g. by stripping a leading `/` before passing to `run_request`) or using a relative path here so the base URL path prefix is preserved.</comment>
<file context>
@@ -0,0 +1,19 @@
+ def list
+ @connection.call(
+ :GET,
+ '/',
+ type: Qdrant::Models::VersionInfo
+ )
</file context>
| ) | ||
| end | ||
|
|
||
| def cluster_get(collection_name:) |
Contributor
There was a problem hiding this comment.
P2: The Collections class exposes duplicate endpoint methods that perform identical HTTP calls with no discernible difference: cluster and cluster_get are both GET /collections/{collection_name}/cluster, and cluster_post and cluster_post_1 are both POST to the same path with identical arguments, query, and body handling. This redundant generated surface creates ambiguity for consumers about which method to call and increases maintenance cost. Please confirm whether these are intentional aliases driven by operationId routing, or if the generator is inadvertently emitting duplicates due to naming collision handling. If unintended, the generator logic should deduplicate equivalent operations so only one method is emitted per unique HTTP verb + path combination.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/collections.rb, line 38:
<comment>The `Collections` class exposes duplicate endpoint methods that perform identical HTTP calls with no discernible difference: `cluster` and `cluster_get` are both GET `/collections/{collection_name}/cluster`, and `cluster_post` and `cluster_post_1` are both POST to the same path with identical arguments, query, and body handling. This redundant generated surface creates ambiguity for consumers about which method to call and increases maintenance cost. Please confirm whether these are intentional aliases driven by operationId routing, or if the generator is inadvertently emitting duplicates due to naming collision handling. If unintended, the generator logic should deduplicate equivalent operations so only one method is emitted per unique HTTP verb + path combination.</comment>
<file context>
@@ -0,0 +1,139 @@
+ )
+ end
+
+ def cluster_get(collection_name:)
+ @connection.call(
+ :GET,
</file context>
| json_key: 'collection', | ||
| required: true | ||
|
|
||
| def initialize(**attrs) |
Contributor
There was a problem hiding this comment.
P2: The model declares collection with required: true, but the constructor does not enforce it, so an invalid object can be created silently and only fail later when valid? is called. Consider triggering validation inside the constructor (or build/from_hash) so required is actually enforced at creation time.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/init_from.rb, line 20:
<comment>The model declares `collection` with `required: true`, but the constructor does not enforce it, so an invalid object can be created silently and only fail later when `valid?` is called. Consider triggering validation inside the constructor (or `build`/`from_hash`) so `required` is actually enforced at creation time.</comment>
<file context>
@@ -0,0 +1,25 @@
+ json_key: 'collection',
+ required: true
+
+ def initialize(**attrs)
+ attrs.each { |k, v| public_send("#{k}=", v) if respond_to?("#{k}=") }
+ end
</file context>
| json_key: 'on_disk', | ||
| required: false | ||
|
|
||
| def initialize(**attrs) |
Contributor
There was a problem hiding this comment.
P2: The initialize method silently ignores any unknown attribute keys (if respond_to?("#{k}=")), which means typos or newly introduced fields are dropped without feedback. In an API client this can mask bugs and cause silent data loss (e.g., a caller passes on_diskk: but the misspelled attribute is ignored and the default is sent instead). Consider raising on unknown keys or capturing them in an additional_properties hash so callers get clear feedback when they pass invalid attributes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/integer_index_params.rb, line 40:
<comment>The `initialize` method silently ignores any unknown attribute keys (`if respond_to?("#{k}=")`), which means typos or newly introduced fields are dropped without feedback. In an API client this can mask bugs and cause silent data loss (e.g., a caller passes `on_diskk:` but the misspelled attribute is ignored and the default is sent instead). Consider raising on unknown keys or capturing them in an `additional_properties` hash so callers get clear feedback when they pass invalid attributes.</comment>
<file context>
@@ -0,0 +1,45 @@
+ json_key: 'on_disk',
+ required: false
+
+ def initialize(**attrs)
+ attrs.each { |k, v| public_send("#{k}=", v) if respond_to?("#{k}=") }
+ end
</file context>
n-rodriguez
added a commit
to n-rodriguez/openapi-generator
that referenced
this pull request
Jul 7, 2026
…time bugs From the automated review on PR OpenAPITools#24220: - Faraday only accepts lowercase HTTP method symbols; normalize the method in Connection#call so operations no longer raise ArgumentError on every real call. - Percent-encode path parameters (ERB::Util.url_encode) instead of raw to_s. - Apply authentication PER REQUEST instead of baking it once into the persistent Faraday connection, so access_token / api_key changes (token refresh, credential rotation) take effect on subsequent requests. - Honour the apiKey location: header / query / cookie (not always a header). - Only materialize an OpenAPI `default` for REQUIRED properties; optional fields stay nil so Serializable#to_hash omits them and the server default applies. - Harden the generated GitHub CI workflow with `permissions: contents: read`. Templates only; both samples regenerated, rspec green (16/0, 577/0) and rubocop clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Thanks for the automated review — went through all 40 findings. Pushed fixes for the genuine runtime bugs and want to explain the ones left as-is. Fixed (commit pushed):
Left as designed (with reasoning):
Both samples regenerate green (petstore 16/0, qdrant 577/0) and pass |
Contributor
There was a problem hiding this comment.
2 issues found across 30 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/metrics.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/metrics.rb:12">
P1: Passing an uppercase symbol `:GET` to `@connection.call` will raise `ArgumentError: unknown http method: :GET` at runtime. `Connection#call` forwards the method directly to Faraday's `run_request`, which only accepts lowercase symbols (`:get`, `:post`, etc.). Consider normalizing the method in `Connection#call` (e.g., `method.to_s.downcase.to_sym`) or updating the generator template to emit lowercase symbols.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/geo_line_string.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/geo_line_string.rb:20">
P2: The model initializer silently ignores unknown keyword arguments and does not enforce required attributes, which can mask caller typos and allow invalid instances to be created. The `initialize` method filters attributes with `respond_to?("#{k}=")`, so a typo like `point:` instead of `points:` is silently dropped. Although `points` is marked `required: true`, the constructor never validates presence or calls the `valid?` method provided by `Qdrant::Validations`. This defers errors to the server or hides them entirely, making debugging difficult for consumers of the generated client. Consider raising `ArgumentError` for unknown keys and invoking required-field validation during initialization or serialization.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/geo_radius.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/geo_radius.rb:25">
P2: The model `initialize` silently ignores unknown keyword arguments, which hides caller typos and makes debugging harder. For example, passing `raius:` instead of `radius:` leaves `radius` as `nil` with no error, only failing later during validation or serialization.
The same codebase already provides a stricter `build` method in the `Validations` mixin that raises `NoMethodError` for unknown keys. Consider making `initialize` consistent by removing the `respond_to?` guard (or explicitly raising `ArgumentError` for unknown attributes) so typos fail fast at construction time.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/ruby-idiomatic/client.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/ruby-idiomatic/client.mustache:13">
P2: The `Client` template emits namespace accessors as raw `def {{name}}` methods without guarding against collisions with the class's own API (`initialize`, `configuration`, `connection`) or inherited Ruby methods. If a generated namespace name matches one of these (e.g. `connection`), `client.connection` will silently return a sub-client instead of the `Connection` instance, breaking any code that relies on the transport object. Consider maintaining a reserved-method list for namespace accessor names (similar to the existing `RESERVED_MODEL_NAMES` guard for models) and renaming or prefixing colliding accessors in the codegen.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/create_alias.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/create_alias.rb:25">
P2: The model initializer silently ignores unknown keys and does not enforce required fields, allowing invalid objects to be created without early failure. The `initialize` method filters kwargs with `respond_to?("#{k}=")`, so typos (e.g., `aliasname:`) are silently dropped. It also does not validate that `required: true` fields like `collection_name` and `alias_name` are present. Unless `Qdrant::Validations` is automatically run on every instance before serialization, callers will only discover the error much later—usually when the API rejects the request. Consider either raising on unknown keys or automatically invoking presence/validations in `initialize`.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/init_from.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/init_from.rb:20">
P2: The model declares `collection` with `required: true`, but the constructor does not enforce it, so an invalid object can be created silently and only fail later when `valid?` is called. Consider triggering validation inside the constructor (or `build`/`from_hash`) so `required` is actually enforced at creation time.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/root.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/root.rb:13">
P2: This API endpoint uses an absolute path `'/'`, which Faraday resolves by replacing the entire path component of the configured `base_url`. If the server URL includes a non-root base path (e.g. `https://host/api`), this request is sent to `https://host/` instead of `https://host/api/`. Consider making paths relative in `Connection#call` (e.g. by stripping a leading `/` before passing to `run_request`) or using a relative path here so the base URL path prefix is preserved.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/discover_request_batch.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/discover_request_batch.rb:20">
P2: `searches` is declared as `required: true`, but the `initialize` method only assigns whatever keyword arguments are provided without enforcing that required attributes are present. Consider validating required fields during construction (or invoking validation from `initialize`) so that missing required attributes fail immediately with a clear error rather than surfacing later during serialization or request execution.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/cluster_status.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/cluster_status.rb:19">
P2: The `oneOf` wrapper uses a first-match-wins strategy that violates OpenAPI `oneOf` semantics. The `build` method returns the first candidate that successfully casts without verifying that no other candidates also match. If a payload is ambiguous and satisfies multiple schemas, the code silently deserializes to whichever candidate is listed first, which can mask data-modeling bugs and propagate incorrect type interpretation. For proper `oneOf` compliance, the wrapper should collect all matching candidates and raise an error (or otherwise signal ambiguity) when more than one matches.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/app_features_telemetry.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/app_features_telemetry.rb:35">
P2: The `initialize` method silently ignores unknown keyword arguments because of the `respond_to?("#{k}=")` guard, so a typo like `AppFeaturesTelemetry.new(debg: true)` passes without error and simply leaves `debug` as `nil`. For a model where every field is declared `required: true`, this is a significant debuggability issue. Consider raising `ArgumentError` for unknown keys, and validating required fields before the object is considered fully initialized.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/create_alias_operation.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/create_alias_operation.rb:20">
P2: The model declares `create_alias` as `required: true`, but the constructor does not enforce its presence, so the object can be instantiated in an invalid state and the nil value is serialized into API requests. Consider invoking validation in `initialize` (e.g., calling `valid?` or raising if required attributes are missing) so that `required: true` is enforced at the point of object construction.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/ruby-idiomatic/configuration.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/ruby-idiomatic/configuration.mustache:13">
P2: The `Configuration` initializer silently drops unknown options due to the `if respond_to?("#{k}=")` guard. Misspelled keys (e.g., `baseurl:`, `acess_token:`) are ignored without feedback, so users get defaults instead of their intended values and may face hard-to-diagnose runtime failures. Consider removing the guard so unrecognized options raise immediately, or explicitly raise an `ArgumentError` with the unknown key name.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/count_request.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/count_request.rb:31">
P2: The initializer silently discards unknown keyword arguments because of the `if respond_to?` guard. In a request model like `CountRequest`, this means a simple caller typo (e.g., `excat: false`) gets dropped without error, and the `@exact = true` default then accidentally applies—producing an unintended API payload with no warning. A fail-fast approach would surface these mistakes immediately during model construction rather than allowing a subtly incorrect request to reach the API.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/ruby-idiomatic/api.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/ruby-idiomatic/api.mustache:14">
P2: Required OpenAPI parameters are not guarded against explicit `nil` values at runtime. Ruby keyword arguments without defaults prevent omission, but passing `nil` explicitly is still allowed and silently produces invalid requests: path params become empty string segments (`nil.to_s`), query/header/form entries are dropped by `Connection#encode_query`/`merge_headers`/`wrap_form` via `.compact`, and body params pass through as `nil`. This violates the OpenAPI required contract and leads to cryptic server errors. Consider adding runtime `nil` checks (e.g., `raise ArgumentError` or a validation helper) for required params before calling `@connection.call`.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/ruby-idiomatic/connection.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/ruby-idiomatic/connection.mustache:21">
P1: `Connection#call` unconditionally invokes `@configuration.apply_auth(request_headers, request_query)` on every request, and the generated API methods in `api.mustache` do not pass any operation-level security context (such as `auth_names` or whether the operation is public). Consequently, `Configuration#apply_auth` applies **all** configured credentials (every apiKey, bearer token, and basic auth pair) whenever they are non-nil, regardless of the OpenAPI operation's `security` requirements.
This can leak API keys or bearer tokens to endpoints declared with `security: []`, and can send multiple conflicting `Authorization` headers to endpoints that only expect one of several alternative schemes. Consider threading operation-level security requirements from the generated API methods into `Connection#call` so that `apply_auth` (or the call site) can selectively enable only the auth schemes relevant to the current operation.</violation>
<violation number="2" location="modules/openapi-generator/src/main/resources/ruby-idiomatic/connection.mustache:77">
P1: The `deserialize` method unconditionally parses String bodies as JSON without rescuing `JSON::ParserError`, which means successful responses containing non-JSON payloads (e.g., plain text, whitespace-only strings, or HTML from a misbehaving server) will raise an unhandled exception that bypasses the `ApiError` wrapping in `call` — breaking the single-choke-point error contract. Consider rescuing `JSON::ParserError` in `deserialize` (or `call`) and re-raising it as an `ApiError` so consumers only need to rescue one exception type.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/cluster.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/cluster.rb:14">
P2: The `recover` method deserializes the `POST /cluster/recover` response into `Qdrant::Models::CreateShardKey200Response`, which is named for a different operation (shard-key creation) and looks like a response-type mapping error. If the actual cluster-recovery schema differs, callers could hit deserialization failures or silently drop fields. Please verify the OpenAPI response schema for this operation and update the bound model accordingly.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/collection_info.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/collection_info.rb:60">
P2: The constructor silently drops unknown keys, which removes normal Ruby keyword-argument safety. A typo or schema drift leaves required attributes nil without any immediate feedback, and since `Qdrant::Validations` is only checked manually (via `valid?`), the invalid object can be serialized and used unless the caller remembers to validate. Consider raising `ArgumentError` for unknown attributes so typos fail fast.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/binary_quantization.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/binary_quantization.rb:20">
P2: The `BinaryQuantization` model marks `binary` as `required: true`, but `initialize` does not enforce that required attributes are present or non-nil. This allows invalid instances to be created silently (for example, `BinaryQuantization.new` or `BinaryQuantization.new(binary: nil)` succeeds even though `binary` is required), and validation is only discoverable later through an explicit `valid?` check. Consider enforcing required fields during initialization—either by validating after assignment in `initialize` or by adding a `validate!` mechanism that runs automatically—so that `required: true` in the DSL actually guarantees a valid instance at construction time.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/discover_request.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/discover_request.rb:73">
P2: The generated model initializer silently ignores unknown keyword arguments, which can mask caller typos or schema drift. Since `Validations.build` already raises `NoMethodError` for unknown keys and `from_hash` handles deserialization strictly, `initialize` should be strict too rather than silently dropping mistyped attributes like `limt:`. Removing the `respond_to?` guard makes construction fail fast and consistent with the rest of the DSL.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/delete_alias.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/delete_alias.rb:20">
P2: A required field (`alias_name`) is declared but never enforced during object construction. Since `initialize` simply assigns whatever keys are provided, callers can create an invalid `DeleteAlias` instance (e.g., via a typo or by omitting the argument entirely) without any immediate error. The invalid state then propagates through serialization (`to_hash` includes `nil` for required fields) and only surfaces as a server-side failure later. Consider validating required fields inside `initialize` or at least invoking the existing `list_invalid_properties` logic so the client fails fast.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/match.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/match.rb:27">
P2: The `anyOf` wrapper silently returns `nil` when no candidate type matches the input, which can hide API payload drift or unexpected data shapes. Consider whether the generator (or this specific wrapper) should raise a descriptive deserialization error instead of defaulting to `nil`, so callers fail fast rather than encountering deferred `NoMethodError`s downstream.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/integer_index_params.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/integer_index_params.rb:40">
P2: The model marks `type` as `required: true`, but `initialize` never validates that required attributes are present. An instance created without `type` will only fail later—often as a cryptic API error—instead of failing fast at construction time. After assigning attributes, call `valid?` (or `list_invalid_properties`) and raise an `ArgumentError` so missing required fields are caught immediately.</violation>
<violation number="2" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/integer_index_params.rb:40">
P2: The `initialize` method silently ignores any unknown attribute keys (`if respond_to?("#{k}=")`), which means typos or newly introduced fields are dropped without feedback. In an API client this can mask bugs and cause silent data loss (e.g., a caller passes `on_diskk:` but the misspelled attribute is ignored and the default is sent instead). Consider raising on unknown keys or capturing them in an `additional_properties` hash so callers get clear feedback when they pass invalid attributes.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/configuration.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/configuration.rb:13">
P2: Unknown configuration keys are silently dropped, which makes typos (e.g., `timeuot:`, `accessTokn:`) fail open with no feedback and leaves defaults active. This can cause hard-to-diagnose misconfiguration bugs at runtime. Consider either failing fast on unknown keys or warning the caller when an option is unrecognized.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/is_empty_condition.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/is_empty_condition.rb:20">
P2: The model constructor does not enforce `required: true` constraints, and serialization proceeds without validation, so instances missing required fields can still be sent to the API as invalid payloads. Consider validating required attributes before serialization or at least during initialization so that missing required fields fail fast on the client side rather than producing a server error.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/field_condition.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/field_condition.rb:51">
P2: The `initialize` method silently ignores unknown keys and does not enforce required fields at construction time. Because it only assigns values when `respond_to?("#{k}=")` is true, a typo like `:kee` instead of `:key` is dropped without error. Similarly, the required `key` attribute is not validated during initialization, so `FieldCondition.new` can create an instance missing mandatory data. This can mask caller mistakes and lead to invalid request payloads. Consider raising `ArgumentError` for unrecognized keys and validating required attributes inside `initialize`, or at least documenting that validation must be triggered explicitly.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/collections_telemetry.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/collections_telemetry.rb:26">
P2: `number_of_collections` is declared `required: true`, but `initialize` performs only best-effort setter assignment and never enforces requiredness. The `Validations` mixin provides `valid?` (which checks the `required` rule), yet `initialize`, `to_hash`, and `Connection#call` never invoke it, so an instance missing a required field can be serialized and sent to the API without any client-side pushback. Consider wiring validation into the construction or serialization path (e.g., calling `valid?` after attribute assignment in `initialize` or before serialization in the connection layer) so required fields are actually enforced.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/context_query.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/context_query.rb:21">
P2: The `initialize` method silently ignores unknown keyword arguments, which masks caller typos and can produce subtly invalid objects (especially for optional fields). The `Validations` module’s `build` method raises on unknown keys, but `initialize` does not, so there is no consistent fail-fast behavior. Consider tightening the generator template (`partial_model_generic.mustache`) to raise on unknown keys—for example by replacing the conditional with an explicit `ArgumentError`—so that typos fail immediately instead of being swallowed.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/collections.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/api/collections.rb:38">
P2: The `Collections` class exposes duplicate endpoint methods that perform identical HTTP calls with no discernible difference: `cluster` and `cluster_get` are both GET `/collections/{collection_name}/cluster`, and `cluster_post` and `cluster_post_1` are both POST to the same path with identical arguments, query, and body handling. This redundant generated surface creates ambiguity for consumers about which method to call and increases maintenance cost. Please confirm whether these are intentional aliases driven by operationId routing, or if the generator is inadvertently emitting duplicates due to naming collision handling. If unintended, the generator logic should deduplicate equivalent operations so only one method is emitted per unique HTTP verb + path combination.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/collection_description.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/collection_description.rb:20">
P2: The model declares `name` as `required: true`, but `initialize` does not enforce that required fields are present. This allows invalid instances to be created silently; downstream serialization or request building can fail with nil errors far from the origin. Consider validating required attributes inside `initialize` (or via a post-init hook) so that `required: true` is actually enforced at object construction time rather than only when `valid?` is called explicitly.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/ruby-idiomatic/validations.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/ruby-idiomatic/validations.mustache:79">
P2: The `validate_attribute` method performs numeric comparisons (`value > rules[:maximum]` and `value < rules[:minimum]`) without first checking that `value` is a comparable numeric type. Because `coerce` in `from_hash` can return raw untyped data and setters are plain `attr_accessor`, a String or other non-numeric assigned to a numeric field causes `valid?` to raise `ArgumentError` instead of returning false. Similarly, `max_items`/`min_items` call `value.length` without confirming the value is a collection, which can raise `NoMethodError`. Consider guarding these checks with type/respond-to guards so that `valid?` remains a safe, exception-free query.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant.rb:28">
P1: The `qdrant.rb` loader setup is missing the `collapse` call that its own comment says is required when the `apiNamespace` wrapper is disabled. Because the generated files under `lib/qdrant/api/` do not define a `Qdrant::Api` module, Zeitwerk will infer the wrong constant paths and autoloading will break. Consider adding `@loader.collapse(...)` before `@loader.setup` (or generating it conditionally in the template) so the directory layout matches the actual module nesting.</violation>
</file>
<file name="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/discover_input_context.rb">
<violation number="1" location="samples/client/others/ruby-idiomatic-qdrant/lib/qdrant/models/discover_input_context.rb:18">
P2: The anyOf `build` loop assumes `Qdrant::Polymorphism.cast` returns `nil` on mismatch, but if a non-matching candidate raises an exception (e.g., `NoMethodError` when a model's `from_hash` receives an Array instead of a Hash), the loop aborts and later candidates are never tried. This breaks the intended "first candidate that validates" fallback behavior. Consider rescuing exceptions during candidate evaluation so mismatching candidates gracefully return `nil` instead of crashing the entire anyOf resolution.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/ruby-idiomatic/partial_model_generic.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/ruby-idiomatic/partial_model_generic.mustache:29">
P1: Optional schema defaults are silently dropped during model initialization. The codegen (`RubyIdiomaticClientCodegen.java`) sets `x-rb-default` for every property that declares a default value, including optional ones, but the template now only applies defaults inside a `{{#required}}` block. As a result, optional properties with schema defaults will leave the backing instance variable as `nil` rather than the documented default. This diverges from both the existing Ruby generator and the Crystal idiomatic generator this work is modeled on. Consider removing the `{{#required}}` wrapper so defaults are applied to all applicable properties.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| method = method.to_s.downcase.to_sym | ||
| request_headers = merge_headers(headers) | ||
| request_query = query.compact.transform_keys(&:to_s) | ||
| @configuration.apply_auth(request_headers, request_query) |
Contributor
There was a problem hiding this comment.
P1: Connection#call unconditionally invokes @configuration.apply_auth(request_headers, request_query) on every request, and the generated API methods in api.mustache do not pass any operation-level security context (such as auth_names or whether the operation is public). Consequently, Configuration#apply_auth applies all configured credentials (every apiKey, bearer token, and basic auth pair) whenever they are non-nil, regardless of the OpenAPI operation's security requirements.
This can leak API keys or bearer tokens to endpoints declared with security: [], and can send multiple conflicting Authorization headers to endpoints that only expect one of several alternative schemes. Consider threading operation-level security requirements from the generated API methods into Connection#call so that apply_auth (or the call site) can selectively enable only the auth schemes relevant to the current operation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/ruby-idiomatic/connection.mustache, line 21:
<comment>`Connection#call` unconditionally invokes `@configuration.apply_auth(request_headers, request_query)` on every request, and the generated API methods in `api.mustache` do not pass any operation-level security context (such as `auth_names` or whether the operation is public). Consequently, `Configuration#apply_auth` applies **all** configured credentials (every apiKey, bearer token, and basic auth pair) whenever they are non-nil, regardless of the OpenAPI operation's `security` requirements.
This can leak API keys or bearer tokens to endpoints declared with `security: []`, and can send multiple conflicting `Authorization` headers to endpoints that only expect one of several alternative schemes. Consider threading operation-level security requirements from the generated API methods into `Connection#call` so that `apply_auth` (or the call site) can selectively enable only the auth schemes relevant to the current operation.</comment>
<file context>
@@ -13,7 +13,12 @@ module {{moduleName}}
+ method = method.to_s.downcase.to_sym
request_headers = merge_headers(headers)
+ request_query = query.compact.transform_keys(&:to_s)
+ @configuration.apply_auth(request_headers, request_query)
request_body =
if form
</file context>
| def initialize(**attrs) | ||
| attrs.each { |k, v| public_send("#{k}=", v) if respond_to?("#{k}=") } | ||
| {{#vars}} | ||
| {{#required}} |
Contributor
There was a problem hiding this comment.
P1: Optional schema defaults are silently dropped during model initialization. The codegen (RubyIdiomaticClientCodegen.java) sets x-rb-default for every property that declares a default value, including optional ones, but the template now only applies defaults inside a {{#required}} block. As a result, optional properties with schema defaults will leave the backing instance variable as nil rather than the documented default. This diverges from both the existing Ruby generator and the Crystal idiomatic generator this work is modeled on. Consider removing the {{#required}} wrapper so defaults are applied to all applicable properties.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/ruby-idiomatic/partial_model_generic.mustache, line 29:
<comment>Optional schema defaults are silently dropped during model initialization. The codegen (`RubyIdiomaticClientCodegen.java`) sets `x-rb-default` for every property that declares a default value, including optional ones, but the template now only applies defaults inside a `{{#required}}` block. As a result, optional properties with schema defaults will leave the backing instance variable as `nil` rather than the documented default. This diverges from both the existing Ruby generator and the Crystal idiomatic generator this work is modeled on. Consider removing the `{{#required}}` wrapper so defaults are applied to all applicable properties.</comment>
<file context>
@@ -26,9 +26,11 @@
def initialize(**attrs)
attrs.each { |k, v| public_send("#{k}=", v) if respond_to?("#{k}=") }
{{#vars}}
+{{#required}}
{{#vendorExtensions.x-rb-default}}
@{{name}} = {{{vendorExtensions.x-rb-default}}} if @{{name}}.nil?
</file context>
n-rodriguez
force-pushed
the
ruby-idiomatic
branch
from
1952ce0 to
fd4a79d
Compare
Add `ruby-idiomatic`, a new opt-in Ruby client generator that emits modern, DRY, multi-instance Ruby. It mirrors the Crystal idiomatic redesign as a SEPARATE generator: the existing `ruby` generator, its templates, and its samples are left byte-for-byte unchanged, so users opt in and migrate when they choose. Highlights: - Namespaced sub-clients (`client.dcim.cable_terminations.list`) via a path->route helper (RubyApiRouting) and a hybrid module nesting (moduleName + configurable `apiNamespace` wrapper, resource leaf compact). - A single `Connection#call` choke-point (Faraday) — keyword args everywhere, no `_with_http_info` twins; a delegating `Response` and one rich `ApiError` (all Faraday errors wrapped); per-request auth (header/query/cookie apiKey, basic, bearer) so token refresh/rotation works; a `Configuration#use` Faraday-middleware seam for request signing. - Native multi-instance: a `Client` facade owns a per-instance Configuration/Connection — no global singleton. - Models on a declarative `attribute` DSL + shared `Serializable`/`Validations` mixins; recursive `from_hash` deserializes nested models/arrays/maps/unions/ enums; `additionalProperties` preserved; anyOf/oneOf via a `Polymorphism` helper; array minItems/maxItems validated; numeric enums bare. - Names are always legal Ruby: enum constants and method names derived from digit-leading values/paths get an `N`/`call_` prefix, purely-symbolic enum values (telephony IVR keys `#`/`*`, ...) map to word names, and deeply-nested inline model names are shortened with a deterministic hash suffix to stay under tar's 100-byte path-component limit so `gem build` succeeds. - Class loading via Zeitwerk. A nested `moduleName` (e.g. `Ovh::Api`) is supported: the parent modules are predefined, the loader is driven explicitly, the gemspec reads VERSION by regex, and version.rb is required rather than autoloaded. Acronym names in both model and API resource classes (HTTPConfig, IPRestriction, DedicatedCloud::TwoFAWhitelist, ...) register explicit Zeitwerk inflections from the gem entrypoint so autoloading resolves the real constant instead of raising Zeitwerk::NameError. - Faraday only; form/multipart (incl. file upload); Ruby 3.0 floor; frozen_string_literal on every generated Ruby file; single-quoted strings; path params percent-encoded; FeatureSet declared honestly (JSON only, no XML, allOf/anyOf/oneOf/Union, apiKey/basic/bearer). - Generated gem passes RuboCop clean out of the box: a real `.rubocop.yml` (rubocop-performance/rake/rspec), a modern gemspec with configurable gem* options (author/homepage/summary/license, MIT-aware LICENSE), LICENSE, dev deps + binstubs, `.rspec`, spec_helper (SimpleCov), a commented `.gitignore`, a GitHub Actions CI workflow, and meaningful RSpec. Samples: petstore and a real-world Qdrant client, both green and rubocop-clean. Codegen unit tests (RubyIdiomaticClientCodegenTest, RubyApiRoutingTest) pass. A samples CI workflow and the generated generator docs are included. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
n-rodriguez
force-pushed
the
ruby-idiomatic
branch
from
fd4a79d to
0ed12e5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a new opt-in Ruby client generator,
ruby-idiomatic, that emits modern, DRY, multi-instance Ruby. It mirrors the recent Crystal idiomatic redesign (#24070) but as a separate generator — the existingrubygenerator, its templates, and its samples are left byte-for-byte unchanged, so there is zero blast radius for its install base. Users opt in with-g ruby-idiomaticand migrate when they choose.What it produces
client.dcim.cable_terminations.listinstead of a flatDcimApiwith prefixed methods (path→route helper + a hybridmodulenesting with a configurableapiNamespacewrapper).Connection#callchoke-point (Faraday) — short declarative operations, keyword args everywhere, no_with_http_infotwins.Response(.data/.status/.headers) and one richApiError(all Faraday errors wrapped).Clientfacade owns a per-instanceConfiguration/Connection; no global singleton.Configuration#useseam to register custom Faraday middleware (request signing à la OVH / AWS SigV4 that a static OpenAPI security scheme cannot model) without subclassing.attributeDSL + sharedSerializable/Validationsmixins (replaces theEnumAttributeValidatorhierarchy and per-model duplication); recursivefrom_hashdeserializes nested models/arrays/maps/unions/enums into typed objects;additionalPropertiespreserved; anyOf/oneOf via aPolymorphismhelper.frozen_string_literaleverywhere; FeatureSet declared honestly (JSON only, no XML, allOf/anyOf/oneOf/Union, apiKey/basic/bearer)..rspec, cleanspec_helper, GitHub Actions CI, and a rich README (incl. the request-signing middleware example). The generated gem passesrubocopclean out of the box (no-Aneeded).Testing / evidence
RubyIdiomaticClientCodegenTest+RubyApiRoutingTest(19 tests) — green.petstore(16 examples, 0 failures) and a real-world Qdrant client undersamples/client/others/ruby-idiomatic-qdrant(577 examples, 0 failures; exercises anyOf / named enums). Both passrubocopwith no offenses.rubygenerator.ruby*samples are untouched.PR checklist
./mvnw clean package, regenerated this generator's samples (./bin/generate-samples.sh bin/configs/ruby-idiomatic*.yaml) and its docs. Only the newruby-idiomaticgenerator's outputs change; no other generator output is affected.cc @wing328 — new opt-in Ruby generator (idiomatic redesign), following up on the Crystal work in #24070.
Summary by cubic
Adds an opt-in Ruby client generator,
ruby-idiomatic, that builds modern, namespaced, multi-instance clients onfaraday+zeitwerk. Includes generator docs (beta), unit tests, and sample clients (petstore and Qdrant) with CI and a samples workflow running petstore RSpec on Ruby 3.0/3.3.New Features
client.collections.points.search).Connection#call(faraday); keyword args only; no_with_http_info.Response(data/status/headers) and richApiErrorwrapping Faraday errors.Configuration/Connection; no globals.Configuration#useto register custom Faraday middleware.attributeDSL with sharedSerializable/Validations; anyOf/oneOf viaPolymorphism; preservesadditionalProperties.faraday,faraday-multipart,zeitwerk.ruby-idiomatic (beta); adds a samples workflow to run petstore RSpec on Ruby 3.0/3.3.Bug Fixes
Connection#call.toApiFilenamefor consistent output and manifests across platforms.Written for commit 0ed12e5. Summary will update on new commits.