Fix real-world OpenAPI code generation#46
Merged
christianhelle merged 1 commit intoApr 28, 2026
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBumps Zig to 0.16.0 across configs/docs; threads std.Io and std.process.Init through CLI, main, generators, and input loaders; refactors generated HTTP client flow and runtime (Client/ApiResult/RawResponse/SSE); standardizes ArrayList initialization; adds resource-wrapper generation and related tests and docs. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (main/init)
participant Generator as Generator
participant FS as std.Io.Dir
participant GenClient as Generated Client (Client)
participant Http as std.http.Client
participant Server as Remote Server
CLI->>Generator: generateCode(allocator, io, args)
Generator->>FS: create dirs / write files using io
Generator->>GenClient: emit runtime + operation code (uses io)
GenClient->>Http: client.request(method, uri, .{})
Http->>Server: sendBodiless / sendBodyComplete
Server-->>Http: response headers + body
Http->>GenClient: receiveHead(), response.reader(...).readAlloc(...)
GenClient->>Generator: parse/return ApiResult / stream events
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
h0rv
force-pushed
the
fix-real-spec-generation
branch
4 times, most recently
from
64d63be to
aebad2b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/generators/v3.0/modelgenerator.zig (1)
91-95:⚠️ Potential issue | 🟠 Major
$reffields are generated as strings instead of referenced model types.
name: ?[]const u8 = nullis not schema-accurate for object references and will produce incorrect client models. Generate the resolved referenced type name here (keeping optionality rules), not a string field.Based on learnings, "Implement features in unified generators, add comprehensive test coverage for both v2.0 and v3.0 inputs, ensure memory cleanup with
deinit(), and follow established JSON parsing patterns".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/generators/v3.0/modelgenerator.zig` around lines 91 - 95, The .reference branch currently appends a string field "name: ?[]const u8 = null" which is incorrect; instead resolve the referenced model type and append that type (keeping the same optionality semantics used elsewhere in this generator). Update the .reference case to call the existing reference-resolution helper (or add a small resolver like resolveReferenceTypeName) to produce the referenced type name and then use parts.append to output " {name}: {ResolvedType}" with the proper nullable/optional wrapper consistent with the generator (preserve optionality rules used by other branches), and remove the string field emission so object references are generated as the correct model type rather than ?[]const u8.src/models/v3.1/schema.zig (1)
135-140:⚠️ Potential issue | 🟡 Minor
exclusiveMaximum/exclusiveMinimummust be numeric types for OpenAPI 3.1 compliance.In OpenAPI 3.1 (JSON Schema 2020-12),
exclusiveMaximumandexclusiveMinimumare numeric bounds, not booleans. The current?booltyping will silently parse numeric values from real 3.1 specs asnull, causing data loss.Change these fields to
?f64and parse withoptionalFloat, or document the lossy 3.0 backwards-compatibility behavior if keeping them as?bool.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v3.1/schema.zig` around lines 135 - 140, The fields exclusiveMaximum and exclusiveMinimum in the schema struct are incorrectly typed as ?bool which breaks OpenAPI 3.1 (JSON Schema 2020-12) where these are numeric bounds; change their types to ?f64 and update parsing to use optionalFloat (or the project’s equivalent numeric optional parser) instead of the current boolean parser so numeric values are preserved; update any references to exclusiveMaximum/exclusiveMinimum parsing logic to expect f64 and handle nulls as before.src/lib.zig (1)
12-29:⚠️ Potential issue | 🟡 MinorAdd allocator leak detection to the doc example.
The example should include
defer _ = init.gpa.deinit;after obtaining the allocator frominit. Whilestd.process.Inititself does not require explicit cleanup (the runtime manages the Init object lifecycle), thegpaallocator within it must be explicitly deinitialized to detect memory leaks. Update the example as follows:Suggested change
pub fn main(init: std.process.Init) !void { const allocator = init.gpa; defer _ = init.gpa.deinit(); const io = init.io; // rest of the code🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib.zig` around lines 12 - 29, The example main currently grabs the process allocator via init.gpa but doesn't deinitialize it; add a deinit call immediately after obtaining the allocator (i.e., after "const allocator = init.gpa") by inserting a defer that calls init.gpa.deinit (e.g., defer _ = init.gpa.deinit();) so the allocator is cleaned up for leak detection; update the example in the pub fn main(init: std.process.Init) block referencing init, gpa, allocator, and main.src/generators/unified/api_generator.zig (1)
334-349:⚠️ Potential issue | 🟡 MinorPath-parameter substitution should match
{name}, not the bare name.
std.mem.replace(u8, new_path, param, param_type, ...)replaces the bare parameter name. It happens to produce{s}/{d}correctly because the surrounding braces remain, but it will also replace any incidental occurrence of that name elsewhere in the path (e.g., a literal path segment that contains the same substring). Replacing the full{name}token is safer.♻️ Suggested approach
- const size = std.mem.replacementSize(u8, new_path, param, param_type); - const output = try self.allocator.alloc(u8, size); - try allocated_paths.append(self.allocator, output); - _ = std.mem.replace(u8, new_path, param, param_type, output); - new_path = output; + const placeholder = try std.fmt.allocPrint(self.allocator, "{{{s}}}", .{param}); + defer self.allocator.free(placeholder); + const replacement = try std.fmt.allocPrint(self.allocator, "{{{s}}}", .{param_type}); + defer self.allocator.free(replacement); + const size = std.mem.replacementSize(u8, new_path, placeholder, replacement); + const output = try self.allocator.alloc(u8, size); + try allocated_paths.append(self.allocator, output); + _ = std.mem.replace(u8, new_path, placeholder, replacement, output); + new_path = output;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/generators/unified/api_generator.zig` around lines 334 - 349, The path-parameter substitution currently calls std.mem.replace and std.mem.replacementSize with the bare parameter name (variable param), which can accidentally replace substrings elsewhere; change both usages to target the full token with braces (e.g., "{" + param + "}") so replacementSize and std.mem.replace scan for "{name}" rather than "name". Update the loop in the code that contains the for (parameters) |parameter| block, replacing references to param passed into std.mem.replacementSize and std.mem.replace with the brace-wrapped token, keeping param_type and the allocation/append logic unchanged (adjust any variable name if you create a token variable).
🧹 Nitpick comments (8)
src/models/v3.1/schema.zig (2)
248-249: Nit: stray blank line inside struct initializer.Line 249 is an empty line in the middle of the field initializer block. Harmless but inconsistent with the rest of the literal —
zig fmtwill keep it but it adds noise.🧹 Proposed cleanup
.minProperties = optionalInteger(obj.get("minProperties")), - .required = if (required_list.items.len > 0) try required_list.toOwnedSlice(allocator) else null,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v3.1/schema.zig` around lines 248 - 249, Remove the stray blank line inside the struct initializer following the .minProperties = optionalInteger(obj.get("minProperties")) entry so the field initializers remain contiguous and consistent with the rest of the literal (i.e., delete the empty line inside the initializer block where the struct/record for the OpenAPI Schema is being constructed).
16-23: Consider accepting.floatinoptionalInteger.
std.jsonwill surface fractional numbers as.float, but some real-world specs (and lossy JSON encoders) emit integers as floats like5.0. The current switch silently drops these and returnsnull, even though they are losslessly representable asi64.♻️ Proposed fix
fn optionalInteger(value: ?json.Value) ?i64 { const val = value orelse return null; return switch (val) { .integer => |i| i, + .float => |f| blk: { + const truncated: i64 = `@intFromFloat`(f); + break :blk if (`@as`(f64, `@floatFromInt`(truncated)) == f) truncated else null; + }, .number_string => |s| std.fmt.parseInt(i64, s, 10) catch null, else => null, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v3.1/schema.zig` around lines 16 - 23, The optionalInteger function currently ignores json.Value .float cases; update the switch in optionalInteger to handle .float by checking the float is finite and has no fractional component (i.e., the value is an integer like 5.0) and then return it as i64; leave existing .integer and .number_string handling intact and return null for non-integer floats. Specifically, in the switch for fn optionalInteger(value: ?json.Value) ?i64 add a .float => |f| arm that verifies f is representable as an integer (fractional part == 0 and finite) and then casts/returns it as i64, otherwise return null.src/generators/unified/model_generator.zig (1)
70-83:@"..."escape set is narrow — verify against OpenAPI property names you expect.Only
\\,",\n,\r,\tare escaped. Any other ASCII control byte (e.g., NUL0x00,0x01–0x1faside from those three,0x7f) gets emitted verbatim into the generated source, which Zig will reject. Real OpenAPI specs rarely contain such bytes in property names, so this is a defensive concern only — but worth either rejecting or hex-escaping these to keep generation deterministic.♻️ Proposed hardening
'\n' => try self.buffer.appendSlice(self.allocator, "\\n"), '\r' => try self.buffer.appendSlice(self.allocator, "\\r"), '\t' => try self.buffer.appendSlice(self.allocator, "\\t"), - else => try self.buffer.append(self.allocator, c), + else => { + if (c < 0x20 or c == 0x7f) { + try self.buffer.writer(self.allocator).print("\\x{x:0>2}", .{c}); + } else { + try self.buffer.append(self.allocator, c); + } + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/generators/unified/model_generator.zig` around lines 70 - 83, The current byte-escaping loop (for (name) |c| ... switch ...) only handles '\\', '"', '\n', '\r', '\t' and emits any other control bytes verbatim; update that switch in model_generator.zig so any other ASCII control (c <= 0x1F or c == 0x7F) is encoded instead of emitted raw — e.g., append a hexadecimal escape sequence like "\xNN" (or a Zig-compatible "\u{NN}" form) by writing the two hex digits after appending the "\x" prefix using the same buffer helpers (self.buffer.appendSlice / self.buffer.append), leaving existing branches for backslash/quote/newline/etc intact. This ensures property names with NUL or other control bytes are rejected/represented safely in the generated source.src/tests/test_input_loader.zig (1)
305-312: Add documentation clarifying how to enable integration tests via build flag.The
build_info.RUN_INTEGRATION_TESTSfield is correctly wired throughbuild.zig(declared at line 165 and added to all modules). However, consider adding a brief comment in the test file showing the correct flag syntax for contributors:zig build -Drun-integration=true test(note: the flag isrun-integration, notRUN_INTEGRATION_TESTS).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tests/test_input_loader.zig` around lines 305 - 312, Add a short explanatory comment above or near the skipIntegrationTests function showing how to enable integration tests via the build flag: mention the function name skipIntegrationTests and the flag backing field build_info.RUN_INTEGRATION_TESTS, and show the exact command contributors should run (zig build -Drun-integration=true test) while noting the flag name is run-integration (not RUN_INTEGRATION_TESTS).src/cli.zig (1)
14-25: Consider a--help/-hshort-circuit and rejecting unknown flags inside the loop.The current shape gives "OpenAPI spec path or URL required" for
openapi2zig/openapi2zig --help, and the inner loop silently ignores unknown flags (anything not-i/-o/--base-url). Both are minor UX nits — neither blocks the PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli.zig` around lines 14 - 25, The parse function currently treats "openapi2zig --help" as missing spec and silently ignores unknown flags in its argument-parsing loop; update parse to short-circuit on "-h" or "--help" (call printUsage() and return error.InvalidArguments or a dedicated HelpReturned) before checking required positional args, and modify the flag-parsing loop (the code that handles "-i", "-o", and "--base-url") to detect and reject unknown flags by printing an error message (via std.debug.print or process logger) and returning error.InvalidArguments so unrecognized options are not silently ignored; ensure you still handle the "generate" subcommand check and the printUsage() call paths (use the same symbols: parse, printUsage, error.InvalidArguments).build.zig (1)
238-254: Includinggeneratedin the package snapshot may pick up uncommitted/untracked artifacts inconsistently.
git ls-files --cached --others --exclude-standardwill include untracked-but-not-ignored files. Ifgenerated/is.gitignored, it'll be silently dropped from the snapshot; if it isn't, untracked artifacts from ad-hocrun-generate-*invocations will leak in. Consider scoping to--cachedonly forgenerated/, or generating into the snapshot deterministically as part ofpackage_snapshot_step.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build.zig` around lines 238 - 254, The current getPackageSnapshotFiles() uses git ls-files with both --cached and --others which causes the "generated" entry to pick up uncommitted/untracked artifacts inconsistently; change the logic so "generated" is only sourced from the index (use --cached only for the generated path) or remove "generated" from the git query and instead include its deterministic output as part of the package_snapshot_step; update getPackageSnapshotFiles to call getGitOutput accordingly (either two calls: one for all paths with --cached/--others excluding "generated" and a second call for "generated" with --cached, or omit "generated" and ensure package_snapshot_step produces a stable file list).docs/index.html (1)
561-593:getPetByIdexample reflects the new request/response API surface.Note that the status check
response.head.status != .okrejects any non-200 success (e.g., 201/204). The unified generator now usesresult.status.class() != .success, which is broader. This is purely an example, but consider tightening parity with the actual generator output if you'd like.Also applies to: 636-668
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/index.html` around lines 561 - 593, The example in getPetById uses a strict check response.head.status != .ok which only accepts 200; change the condition to check the status class instead (e.g., response.head.status.class() != .success) so any 2xx success is accepted; update the conditional around response.head.status (replacing the .ok comparison) and keep the same error return (error.ResponseError) and surrounding flow in getPetById.src/generators/unified/api_generator.zig (1)
8-39: Consider extracting the identifier helpers into a shared module.
isIdentStart,isIdentContinue,isReservedIdent,isBareIdentifier, andappendIdentifierhere duplicate the same helpers insrc/generators/unified/model_generator.zig. A sharedidentifiers.zigwould avoid the two implementations drifting (e.g., when Zig adds/removes reserved words or you add escape handling for additional control chars).Also applies to: 65-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/generators/unified/api_generator.zig` around lines 8 - 39, The identifier helper functions (isIdentStart, isIdentContinue, isReservedIdent, isBareIdentifier and appendIdentifier) are duplicated; extract them into a shared identifiers.zig module and import it from both generators. Create a new identifiers.zig that exports isIdentStart, isIdentContinue, isReservedIdent, isBareIdentifier, and appendIdentifier, move the reserved-words table there, then remove the duplicate implementations from api_generator.zig and model_generator.zig and replace them with calls to the shared symbols (e.g., identifiers.isBareIdentifier). Ensure visibility/exports match the call sites and update imports/usages accordingly, then run tests/build to validate no API regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/generators/unified/api_generator.zig`:
- Around line 378-385: The generated code embeds parameter.name directly into a
string literal via buffer.appendSlice calls, which will break if the name
contains quotes, backslashes, or control chars; add and reuse a string-literal
escape helper (similar to the escape branches in appendIdentifier) that returns
an escaped slice for a given name, then replace the direct
appendSlice(parameter.name) inside the query-param generation loop with a call
to that helper so the value passed into the quoted literal is safely escaped
before appending and before calling appendQueryParam (symbols to touch:
parameter.name usage in the for loop, buffer.appendSlice calls around the quoted
literal, and implement/reuse the escape helper used by appendIdentifier).
- Around line 310-318: The current loop in the generator treats any parameter
not in .path, .body, or .query as a no-op (`_ = name;`); update the branch
inside the loop over operation.parameters so it handles `.header` and `.cookie`
locations instead of dropping them: for `.header` parameters emit code that adds
the parameter to the request headers (properly formatting the header name and
serializing the parameter value) and for `.cookie` parameters accumulate them
into a Cookie header (concatenate name=value pairs and add/update the Cookie
header once); keep using the existing helpers like self.appendIdentifier and
self.buffer.appendSlice and ensure you remove the `_ =` fallback so
header/cookie params are applied to the outgoing request rather than silently
ignored.
- Around line 100-214: The emitted Client template hardcodes OpenAI-specific
fields/headers (api_key + Bearer auth, OpenAI-Organization, OpenAI-Project) in
the Client type and appendClientHeaders function; change this by removing those
hardcoded headers and making auth configurable: update the Client struct (remove
or make api_key optional) and change appendClientHeaders(allocator, headers,
client, include_content_type) to accept an auth configuration (e.g.,
auth_scheme/name and credential value) or derive it from the OpenAPI
securitySchemes provided to the generator, then build the Authorization header
based on that scheme instead of always using "Bearer {api_key}", and drop the
OpenAI-Organization/OpenAI-Project defaults (or gate them behind a generator
flag like enableOpenAIOpionatedHeaders). Ensure references to api_key,
appendClientHeaders, Client, and the OpenAI header names are updated accordingly
so other call sites supply the new auth/config parameters.
- Around line 467-490: appendZigTypeFromSchema currently erases array element
types by delegating arrays to appendZigTypeFromSchemaType which always emits
"[]const std.json.Value"; change appendZigTypeFromSchema to handle schema.type
== .array by checking schema.items and recursing into appendZigTypeFromSchema
(or a new helper) to build a "[]const <ItemType>" string (mirroring
appendZigType/appendArrayItemType behavior in model_generator.zig), and leave
appendZigTypeFromSchemaType only for cases where you truly only have a
SchemaType (removing the array case from its switch); ensure you use
schema.items when present and fall back to "[]const std.json.Value" only when
items is missing.
- Around line 196-211: The helper appendClientHeaders allocates auth_header then
calls headers.append several times (Authorization, OpenAI-Organization,
OpenAI-Project, default_headers) but if any append fails the function returns an
error without freeing auth_header, causing a leak; modify appendClientHeaders to
free auth_header on all error paths by using errdefer (or equivalent err
handling) immediately after allocation (e.g., right after auth_header is set) so
that if any subsequent headers.append or other operation returns an error the
allocated auth_header is freed before propagating the error, and ensure the
normal success path still returns the auth_header to the caller without
double-freeing.
In `@src/generators/unified/model_generator.zig`:
- Around line 141-145: The current code unconditionally treats any field_name
equal to "model" as a string by appending "[]const u8", which is brittle; change
the branch so the name-based override only applies when the field_schema is
truly free-form/has no concrete type (e.g., no "type" or "$ref" present or
schema.kind == .any). In practice, update the logic around field_name and
field_schema in model_generator.zig: instead of only checking field_name ==
"model", inspect field_schema (via whatever accessors exist on the schema object
used here) to detect absence of concrete typing (no ref/type/enum/struct) and
only then call self.buffer.appendSlice(self.allocator, "[]const u8"); otherwise
call self.appendZigType(field_schema) to emit the real type. Ensure you
reference the same symbols: field_name, field_schema, appendZigType, and
buffer.appendSlice.
In `@src/generators/v2.0/apigenerator.zig`:
- Around line 208-222: The generated request flow stops after
req.sendBodiless()/req.sendBodyComplete(...) and never drives the response;
update the generator (the code building parts around client.request and
req.deinit) to emit a call to req.receiveHead(&buffer) after sending (using the
same buffer variable name used elsewhere), then inspect the received head (e.g.,
check response.status_code for non-success and return an error or handle it) and
only then proceed to read the body or finish; ensure the generated sequence
includes req.receiveHead(&buffer) and any simple status check before leaving the
function so connections are properly completed.
In `@src/generators/v3.0/apigenerator.zig`:
- Around line 191-205: The generated v3.0 client stops after
sendBodyComplete/sendBodiless and never reads the response, so you must add the
missing response lifecycle calls: after try req.sendBodyComplete(...) / try
req.sendBodiless(), call try req.receiveHead() and inspect the returned head
(e.g. head.statusCode) and then loop over req.receiveBodyChunk / try
req.receiveBodyComplete to collect the body (or use the same helpers used in
v2.0), propagate HTTP error statuses, and return the assembled response; update
the code that builds the method around the req variable and client.request(...)
to mirror the reference flow in src/input_loader.zig (lines 60–95) so v3.0 emits
req.receiveHead(), body-chunk handling, and proper error handling just like the
v2.0 generator.
In `@src/generators/v3.0/modelgenerator.zig`:
- Line 45: The .reference arm in modelgenerator.zig currently no-ops
(".reference => {}"), causing top-level components.schemas references to be
dropped; update the .reference case in the switch handling within
modelgenerator.zig to resolve the Reference object and either generate a type
alias or delegate to the unified converter/generator pipeline (use the
converters in src/generators/converters/ and unified generators in
src/generators/unified/) so that referenced schemas produce emitted Zig types
(i.e., look up the target schema by $ref, produce an alias type name matching
the reference key or forward to the unified generator to generate the actual
type).
---
Outside diff comments:
In `@src/generators/unified/api_generator.zig`:
- Around line 334-349: The path-parameter substitution currently calls
std.mem.replace and std.mem.replacementSize with the bare parameter name
(variable param), which can accidentally replace substrings elsewhere; change
both usages to target the full token with braces (e.g., "{" + param + "}") so
replacementSize and std.mem.replace scan for "{name}" rather than "name". Update
the loop in the code that contains the for (parameters) |parameter| block,
replacing references to param passed into std.mem.replacementSize and
std.mem.replace with the brace-wrapped token, keeping param_type and the
allocation/append logic unchanged (adjust any variable name if you create a
token variable).
In `@src/generators/v3.0/modelgenerator.zig`:
- Around line 91-95: The .reference branch currently appends a string field
"name: ?[]const u8 = null" which is incorrect; instead resolve the referenced
model type and append that type (keeping the same optionality semantics used
elsewhere in this generator). Update the .reference case to call the existing
reference-resolution helper (or add a small resolver like
resolveReferenceTypeName) to produce the referenced type name and then use
parts.append to output " {name}: {ResolvedType}" with the proper
nullable/optional wrapper consistent with the generator (preserve optionality
rules used by other branches), and remove the string field emission so object
references are generated as the correct model type rather than ?[]const u8.
In `@src/lib.zig`:
- Around line 12-29: The example main currently grabs the process allocator via
init.gpa but doesn't deinitialize it; add a deinit call immediately after
obtaining the allocator (i.e., after "const allocator = init.gpa") by inserting
a defer that calls init.gpa.deinit (e.g., defer _ = init.gpa.deinit();) so the
allocator is cleaned up for leak detection; update the example in the pub fn
main(init: std.process.Init) block referencing init, gpa, allocator, and main.
In `@src/models/v3.1/schema.zig`:
- Around line 135-140: The fields exclusiveMaximum and exclusiveMinimum in the
schema struct are incorrectly typed as ?bool which breaks OpenAPI 3.1 (JSON
Schema 2020-12) where these are numeric bounds; change their types to ?f64 and
update parsing to use optionalFloat (or the project’s equivalent numeric
optional parser) instead of the current boolean parser so numeric values are
preserved; update any references to exclusiveMaximum/exclusiveMinimum parsing
logic to expect f64 and handle nulls as before.
---
Nitpick comments:
In `@build.zig`:
- Around line 238-254: The current getPackageSnapshotFiles() uses git ls-files
with both --cached and --others which causes the "generated" entry to pick up
uncommitted/untracked artifacts inconsistently; change the logic so "generated"
is only sourced from the index (use --cached only for the generated path) or
remove "generated" from the git query and instead include its deterministic
output as part of the package_snapshot_step; update getPackageSnapshotFiles to
call getGitOutput accordingly (either two calls: one for all paths with
--cached/--others excluding "generated" and a second call for "generated" with
--cached, or omit "generated" and ensure package_snapshot_step produces a stable
file list).
In `@docs/index.html`:
- Around line 561-593: The example in getPetById uses a strict check
response.head.status != .ok which only accepts 200; change the condition to
check the status class instead (e.g., response.head.status.class() != .success)
so any 2xx success is accepted; update the conditional around
response.head.status (replacing the .ok comparison) and keep the same error
return (error.ResponseError) and surrounding flow in getPetById.
In `@src/cli.zig`:
- Around line 14-25: The parse function currently treats "openapi2zig --help" as
missing spec and silently ignores unknown flags in its argument-parsing loop;
update parse to short-circuit on "-h" or "--help" (call printUsage() and return
error.InvalidArguments or a dedicated HelpReturned) before checking required
positional args, and modify the flag-parsing loop (the code that handles "-i",
"-o", and "--base-url") to detect and reject unknown flags by printing an error
message (via std.debug.print or process logger) and returning
error.InvalidArguments so unrecognized options are not silently ignored; ensure
you still handle the "generate" subcommand check and the printUsage() call paths
(use the same symbols: parse, printUsage, error.InvalidArguments).
In `@src/generators/unified/api_generator.zig`:
- Around line 8-39: The identifier helper functions (isIdentStart,
isIdentContinue, isReservedIdent, isBareIdentifier and appendIdentifier) are
duplicated; extract them into a shared identifiers.zig module and import it from
both generators. Create a new identifiers.zig that exports isIdentStart,
isIdentContinue, isReservedIdent, isBareIdentifier, and appendIdentifier, move
the reserved-words table there, then remove the duplicate implementations from
api_generator.zig and model_generator.zig and replace them with calls to the
shared symbols (e.g., identifiers.isBareIdentifier). Ensure visibility/exports
match the call sites and update imports/usages accordingly, then run tests/build
to validate no API regressions.
In `@src/generators/unified/model_generator.zig`:
- Around line 70-83: The current byte-escaping loop (for (name) |c| ... switch
...) only handles '\\', '"', '\n', '\r', '\t' and emits any other control bytes
verbatim; update that switch in model_generator.zig so any other ASCII control
(c <= 0x1F or c == 0x7F) is encoded instead of emitted raw — e.g., append a
hexadecimal escape sequence like "\xNN" (or a Zig-compatible "\u{NN}" form) by
writing the two hex digits after appending the "\x" prefix using the same buffer
helpers (self.buffer.appendSlice / self.buffer.append), leaving existing
branches for backslash/quote/newline/etc intact. This ensures property names
with NUL or other control bytes are rejected/represented safely in the generated
source.
In `@src/models/v3.1/schema.zig`:
- Around line 248-249: Remove the stray blank line inside the struct initializer
following the .minProperties = optionalInteger(obj.get("minProperties")) entry
so the field initializers remain contiguous and consistent with the rest of the
literal (i.e., delete the empty line inside the initializer block where the
struct/record for the OpenAPI Schema is being constructed).
- Around line 16-23: The optionalInteger function currently ignores json.Value
.float cases; update the switch in optionalInteger to handle .float by checking
the float is finite and has no fractional component (i.e., the value is an
integer like 5.0) and then return it as i64; leave existing .integer and
.number_string handling intact and return null for non-integer floats.
Specifically, in the switch for fn optionalInteger(value: ?json.Value) ?i64 add
a .float => |f| arm that verifies f is representable as an integer (fractional
part == 0 and finite) and then casts/returns it as i64, otherwise return null.
In `@src/tests/test_input_loader.zig`:
- Around line 305-312: Add a short explanatory comment above or near the
skipIntegrationTests function showing how to enable integration tests via the
build flag: mention the function name skipIntegrationTests and the flag backing
field build_info.RUN_INTEGRATION_TESTS, and show the exact command contributors
should run (zig build -Drun-integration=true test) while noting the flag name is
run-integration (not RUN_INTEGRATION_TESTS).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c4669d37-a1d7-47d8-bd30-c84247c53c41
⛔ Files ignored due to path filters (6)
generated/compile_generated.zigis excluded by!**/generated/**generated/generated_v2.zigis excluded by!**/generated/**generated/generated_v3.zigis excluded by!**/generated/**generated/generated_v31.zigis excluded by!**/generated/**generated/generated_v32.zigis excluded by!**/generated/**generated/main.zigis excluded by!**/generated/**
📒 Files selected for processing (64)
.devcontainer/README.md.devcontainer/devcontainer.json.github/copilot-instructions.md.github/workflows/ci.yml.github/workflows/release.yml.gitignore.mise.tomlREADME.mdbuild.zigbuild.zig.zondocs/index.htmlexamples/PACKAGE.mdexamples/example_build.zigexamples/example_usage.zigexamples/package_consumer/build.zig.zonsrc/cli.zigsrc/generator.zigsrc/generators/converters/openapi31_converter.zigsrc/generators/converters/openapi32_converter.zigsrc/generators/converters/openapi_converter.zigsrc/generators/unified/api_generator.zigsrc/generators/unified/model_generator.zigsrc/generators/v2.0/apigenerator.zigsrc/generators/v2.0/modelgenerator.zigsrc/generators/v3.0/apigenerator.zigsrc/generators/v3.0/modelgenerator.zigsrc/input_loader.zigsrc/lib.zigsrc/main.zigsrc/models/v2.0/operation.zigsrc/models/v2.0/parameter.zigsrc/models/v2.0/paths.zigsrc/models/v2.0/schema.zigsrc/models/v2.0/security.zigsrc/models/v2.0/swagger.zigsrc/models/v3.0/components.zigsrc/models/v3.0/openapi.zigsrc/models/v3.0/operation.zigsrc/models/v3.0/paths.zigsrc/models/v3.0/schema.zigsrc/models/v3.0/security.zigsrc/models/v3.0/server.zigsrc/models/v3.1/components.zigsrc/models/v3.1/openapi.zigsrc/models/v3.1/operation.zigsrc/models/v3.1/paths.zigsrc/models/v3.1/schema.zigsrc/models/v3.1/security.zigsrc/models/v3.1/server.zigsrc/models/v3.2/components.zigsrc/models/v3.2/openapi.zigsrc/models/v3.2/operation.zigsrc/models/v3.2/paths.zigsrc/models/v3.2/schema.zigsrc/models/v3.2/security.zigsrc/models/v3.2/server.zigsrc/tests/comprehensive_converter_tests.zigsrc/tests/openapi_v31_tests.zigsrc/tests/openapi_v32_tests.zigsrc/tests/openapi_v3_tests.zigsrc/tests/swagger_v2_tests.zigsrc/tests/test_input_loader.zigsrc/tests/test_utils.zigsrc/tests/unified_converter_tests.zig
💤 Files with no reviewable changes (1)
- .gitignore
Comment on lines
+100
to
+214
| try self.buffer.appendSlice(self.allocator, | ||
| \\ | ||
| \\pub fn Owned(comptime T: type) type { | ||
| \\ return struct { | ||
| \\ allocator: std.mem.Allocator, | ||
| \\ body: []u8, | ||
| \\ parsed: std.json.Parsed(T), | ||
| \\ | ||
| \\ pub fn deinit(self: *@This()) void { | ||
| \\ self.parsed.deinit(); | ||
| \\ self.allocator.free(self.body); | ||
| \\ } | ||
| \\ | ||
| \\ pub fn value(self: *@This()) *T { | ||
| \\ return &self.parsed.value; | ||
| \\ } | ||
| \\ }; | ||
| \\} | ||
| \\ | ||
| \\pub const Client = struct { | ||
| \\ allocator: std.mem.Allocator, | ||
| \\ io: std.Io, | ||
| \\ http: std.http.Client, | ||
| \\ api_key: []const u8, | ||
| \\ base_url: []const u8 = " | ||
| ); | ||
| if (self.args.base_url) |base_url| try self.buffer.appendSlice(self.allocator, base_url); | ||
| try self.buffer.appendSlice(self.allocator, | ||
| \\", | ||
| \\ organization: ?[]const u8 = null, | ||
| \\ project: ?[]const u8 = null, | ||
| \\ default_headers: []const std.http.Header = &.{}, | ||
| \\ | ||
| \\ pub fn init(allocator: std.mem.Allocator, io: std.Io, api_key: []const u8) Client { | ||
| \\ return .{ | ||
| \\ .allocator = allocator, | ||
| \\ .io = io, | ||
| \\ .http = .{ .allocator = allocator, .io = io }, | ||
| \\ .api_key = api_key, | ||
| \\ }; | ||
| \\ } | ||
| \\ | ||
| \\ pub fn deinit(self: *Client) void { | ||
| \\ self.http.deinit(); | ||
| \\ } | ||
| \\ | ||
| \\ pub fn withBaseUrl(self: *Client, base_url: []const u8) void { | ||
| \\ self.base_url = base_url; | ||
| \\ } | ||
| \\}; | ||
| \\ | ||
| \\fn isQueryChar(c: u8) bool { | ||
| \\ return std.ascii.isAlphanumeric(c) or switch (c) { | ||
| \\ '-', '.', '_', '~' => true, | ||
| \\ else => false, | ||
| \\ }; | ||
| \\} | ||
| \\ | ||
| \\fn writeQueryComponent(writer: *std.Io.Writer, value: []const u8) !void { | ||
| \\ try std.Uri.Component.percentEncode(writer, value, isQueryChar); | ||
| \\} | ||
| \\ | ||
| \\fn writeQueryValue(writer: *std.Io.Writer, value: anytype) !void { | ||
| \\ const T = @TypeOf(value); | ||
| \\ switch (@typeInfo(T)) { | ||
| \\ .pointer => |ptr| { | ||
| \\ if (ptr.size == .slice and ptr.child == u8) { | ||
| \\ try writeQueryComponent(writer, value); | ||
| \\ } else { | ||
| \\ try std.json.Stringify.value(value, .{}, writer); | ||
| \\ } | ||
| \\ }, | ||
| \\ .int, .comptime_int, .float, .comptime_float, .bool => try writer.print("{}", .{value}), | ||
| \\ .@"enum" => try writeQueryComponent(writer, @tagName(value)), | ||
| \\ else => try std.json.Stringify.value(value, .{}, writer), | ||
| \\ } | ||
| \\} | ||
| \\ | ||
| \\fn appendQueryParam(writer: *std.Io.Writer, first_query: *bool, name: []const u8, value: anytype) !void { | ||
| \\ if (first_query.*) { | ||
| \\ try writer.writeByte('?'); | ||
| \\ first_query.* = false; | ||
| \\ } else { | ||
| \\ try writer.writeByte('&'); | ||
| \\ } | ||
| \\ try writeQueryComponent(writer, name); | ||
| \\ try writer.writeByte('='); | ||
| \\ try writeQueryValue(writer, value); | ||
| \\} | ||
| \\ | ||
| \\fn appendClientHeaders(allocator: std.mem.Allocator, headers: *std.ArrayList(std.http.Header), client: *Client, include_content_type: bool) !?[]u8 { | ||
| \\ if (include_content_type) { | ||
| \\ try headers.append(allocator, .{ .name = "Content-Type", .value = "application/json" }); | ||
| \\ } | ||
| \\ try headers.append(allocator, .{ .name = "Accept", .value = "application/json" }); | ||
| \\ | ||
| \\ var auth_header: ?[]u8 = null; | ||
| \\ if (client.api_key.len > 0) { | ||
| \\ auth_header = try std.fmt.allocPrint(allocator, "Bearer {s}", .{client.api_key}); | ||
| \\ try headers.append(allocator, .{ .name = "Authorization", .value = auth_header.? }); | ||
| \\ } | ||
| \\ if (client.organization) |organization| { | ||
| \\ try headers.append(allocator, .{ .name = "OpenAI-Organization", .value = organization }); | ||
| \\ } | ||
| \\ if (client.project) |project| { | ||
| \\ try headers.append(allocator, .{ .name = "OpenAI-Project", .value = project }); | ||
| \\ } | ||
| \\ for (client.default_headers) |header| { | ||
| \\ try headers.append(allocator, header); | ||
| \\ } | ||
| \\ return auth_header; | ||
| \\} | ||
| \\ | ||
| \\ | ||
| ); |
Contributor
There was a problem hiding this comment.
Avoid hardcoding OpenAI-specific concerns in the unified generator.
The emitted Client template hardcodes OpenAI semantics — api_key with Bearer auth, OpenAI-Organization, and OpenAI-Project headers. For a generic OpenAPI generator (Petstore, GitHub, Stripe, etc.) these names are wrong, and the auth choice should ideally be driven by the spec's securitySchemes rather than always emitting a Bearer header. At minimum, consider gating these behind a flag, deriving the auth header name/scheme from the spec, and dropping the OpenAI-prefixed headers from the default template.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/generators/unified/api_generator.zig` around lines 100 - 214, The emitted
Client template hardcodes OpenAI-specific fields/headers (api_key + Bearer auth,
OpenAI-Organization, OpenAI-Project) in the Client type and appendClientHeaders
function; change this by removing those hardcoded headers and making auth
configurable: update the Client struct (remove or make api_key optional) and
change appendClientHeaders(allocator, headers, client, include_content_type) to
accept an auth configuration (e.g., auth_scheme/name and credential value) or
derive it from the OpenAPI securitySchemes provided to the generator, then build
the Authorization header based on that scheme instead of always using "Bearer
{api_key}", and drop the OpenAI-Organization/OpenAI-Project defaults (or gate
them behind a generator flag like enableOpenAIOpionatedHeaders). Ensure
references to api_key, appendClientHeaders, Client, and the OpenAI header names
are updated accordingly so other call sites supply the new auth/config
parameters.
Comment on lines
310
to
318
| if (operation.parameters) |parameters| { | ||
| if (parameters.len > 0) { | ||
| for (parameters) |parameter| { | ||
| if (parameter.location != .path and parameter.location != .body) { | ||
| try self.buffer.appendSlice(self.allocator, " _ = "); | ||
| try self.buffer.appendSlice(self.allocator, parameter.name); | ||
| try self.buffer.appendSlice(self.allocator, ";\n"); | ||
| } | ||
| for (parameters) |parameter| { | ||
| if (parameter.location != .path and parameter.location != .body and parameter.location != .query) { | ||
| try self.buffer.appendSlice(self.allocator, " _ = "); | ||
| try self.appendIdentifier(parameter.name); | ||
| try self.buffer.appendSlice(self.allocator, ";\n"); | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Header and cookie parameters are silently dropped.
Parameters whose location is neither path, body, nor query are emitted as _ = name; rather than being applied to the request. Header parameters should be appended to headers (with proper formatting), and cookie parameters should be combined into a Cookie header. As written, generated clients silently ignore header/cookie inputs, which breaks any spec that requires them (very common in real-world OpenAPI specs).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/generators/unified/api_generator.zig` around lines 310 - 318, The current
loop in the generator treats any parameter not in .path, .body, or .query as a
no-op (`_ = name;`); update the branch inside the loop over operation.parameters
so it handles `.header` and `.cookie` locations instead of dropping them: for
`.header` parameters emit code that adds the parameter to the request headers
(properly formatting the header name and serializing the parameter value) and
for `.cookie` parameters accumulate them into a Cookie header (concatenate
name=value pairs and add/update the Cookie header once); keep using the existing
helpers like self.appendIdentifier and self.buffer.appendSlice and ensure you
remove the `_ =` fallback so header/cookie params are applied to the outgoing
request rather than silently ignored.
Comment on lines
+208
to
222
| try parts.append(allocator, "\n"); | ||
| try parts.append(allocator, " var req = try client.request(."); | ||
| try parts.append(allocator, method); | ||
| try parts.append(allocator, ", uri, .{\n"); | ||
| try parts.append(allocator, | ||
| \\ .server_header_buffer = buf, | ||
| \\ }); | ||
| \\ defer req.deinit(); | ||
| \\ | ||
| \\ try req.send(); | ||
| ); | ||
| try parts.append(allocator, ", uri, .{});\n"); | ||
| try parts.append(allocator, " defer req.deinit();\n"); | ||
| if (has_request_body) { | ||
| try parts.append(allocator, "\n\n"); | ||
| try parts.append(allocator, " var str = std.ArrayList([]const u8){};\n"); | ||
| try parts.append(allocator, " defer str.deinit(allocator);\n\n"); | ||
| try parts.append(allocator, " try std.json.stringify(requestBody, .{}, str.writer());\n"); | ||
| try parts.append(allocator, " const body = try std.mem.join(allocator, \"\", str.items);\n"); | ||
| try parts.append(allocator, " defer allocator.free(body);\n\n"); | ||
| try parts.append(allocator, " req.transfer_encoding = .{ .content_length = body.len };\n"); | ||
| try parts.append(allocator, " try req.writeAll(body);\n\n"); | ||
| } else { | ||
| try parts.append(allocator, "\n"); | ||
| try parts.append(allocator, " var str: std.Io.Writer.Allocating = .init(allocator);\n"); | ||
| try parts.append(allocator, " defer str.deinit();\n\n"); | ||
| try parts.append(allocator, " try std.json.Stringify.value(requestBody, .{}, &str.writer);\n"); | ||
| try parts.append(allocator, " const body = str.written();\n\n"); | ||
| try parts.append(allocator, " try req.sendBodyComplete(body);\n"); | ||
| } else { | ||
| try parts.append(allocator, "\n try req.sendBodiless();\n"); | ||
| } |
Contributor
There was a problem hiding this comment.
Generated request lifecycle is incomplete — receiveHead is never emitted.
The reference implementation in src/input_loader.zig (lines 60–95) shows the required Zig 0.16 flow: after sendBodiless() / sendBodyComplete(...), callers must invoke req.receiveHead(&buffer) to actually drive the response. The generated code emitted here stops at sending, so generated client methods will:
- never observe HTTP status codes (5xx/4xx silently appear successful),
- never read the response body, and
- potentially close the connection in an indeterminate state under
defer req.deinit().
Even though the generated functions are typed !void, the response side of the exchange still needs to be consumed for the request to be considered "executed". Please emit receiveHead (and ideally a status check) after sendBodiless / sendBodyComplete.
🐛 Suggested generated-output shape (sketch)
- try parts.append(allocator, " try req.sendBodyComplete(body);\n");
- } else {
- try parts.append(allocator, "\n try req.sendBodiless();\n");
- }
+ try parts.append(allocator, " try req.sendBodyComplete(body);\n");
+ } else {
+ try parts.append(allocator, "\n try req.sendBodiless();\n");
+ }
+ try parts.append(allocator, "\n var redirect_buffer: [1024]u8 = undefined;\n");
+ try parts.append(allocator, " var response = try req.receiveHead(&redirect_buffer);\n");
+ try parts.append(allocator, " _ = &response;\n");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try parts.append(allocator, "\n"); | |
| try parts.append(allocator, " var req = try client.request(."); | |
| try parts.append(allocator, method); | |
| try parts.append(allocator, ", uri, .{\n"); | |
| try parts.append(allocator, | |
| \\ .server_header_buffer = buf, | |
| \\ }); | |
| \\ defer req.deinit(); | |
| \\ | |
| \\ try req.send(); | |
| ); | |
| try parts.append(allocator, ", uri, .{});\n"); | |
| try parts.append(allocator, " defer req.deinit();\n"); | |
| if (has_request_body) { | |
| try parts.append(allocator, "\n\n"); | |
| try parts.append(allocator, " var str = std.ArrayList([]const u8){};\n"); | |
| try parts.append(allocator, " defer str.deinit(allocator);\n\n"); | |
| try parts.append(allocator, " try std.json.stringify(requestBody, .{}, str.writer());\n"); | |
| try parts.append(allocator, " const body = try std.mem.join(allocator, \"\", str.items);\n"); | |
| try parts.append(allocator, " defer allocator.free(body);\n\n"); | |
| try parts.append(allocator, " req.transfer_encoding = .{ .content_length = body.len };\n"); | |
| try parts.append(allocator, " try req.writeAll(body);\n\n"); | |
| } else { | |
| try parts.append(allocator, "\n"); | |
| try parts.append(allocator, " var str: std.Io.Writer.Allocating = .init(allocator);\n"); | |
| try parts.append(allocator, " defer str.deinit();\n\n"); | |
| try parts.append(allocator, " try std.json.Stringify.value(requestBody, .{}, &str.writer);\n"); | |
| try parts.append(allocator, " const body = str.written();\n\n"); | |
| try parts.append(allocator, " try req.sendBodyComplete(body);\n"); | |
| } else { | |
| try parts.append(allocator, "\n try req.sendBodiless();\n"); | |
| } | |
| try parts.append(allocator, "\n"); | |
| try parts.append(allocator, " var req = try client.request(."); | |
| try parts.append(allocator, method); | |
| try parts.append(allocator, ", uri, .{});\n"); | |
| try parts.append(allocator, " defer req.deinit();\n"); | |
| if (has_request_body) { | |
| try parts.append(allocator, "\n"); | |
| try parts.append(allocator, " var str: std.Io.Writer.Allocating = .init(allocator);\n"); | |
| try parts.append(allocator, " defer str.deinit();\n\n"); | |
| try parts.append(allocator, " try std.json.Stringify.value(requestBody, .{}, &str.writer);\n"); | |
| try parts.append(allocator, " const body = str.written();\n\n"); | |
| try parts.append(allocator, " try req.sendBodyComplete(body);\n"); | |
| } else { | |
| try parts.append(allocator, "\n try req.sendBodiless();\n"); | |
| } | |
| try parts.append(allocator, "\n var redirect_buffer: [1024]u8 = undefined;\n"); | |
| try parts.append(allocator, " var response = try req.receiveHead(&redirect_buffer);\n"); | |
| try parts.append(allocator, " _ = &response;\n"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/generators/v2.0/apigenerator.zig` around lines 208 - 222, The generated
request flow stops after req.sendBodiless()/req.sendBodyComplete(...) and never
drives the response; update the generator (the code building parts around
client.request and req.deinit) to emit a call to req.receiveHead(&buffer) after
sending (using the same buffer variable name used elsewhere), then inspect the
received head (e.g., check response.status_code for non-success and return an
error or handle it) and only then proceed to read the body or finish; ensure the
generated sequence includes req.receiveHead(&buffer) and any simple status check
before leaving the function so connections are properly completed.
Comment on lines
+191
to
205
| try parts.append(allocator, "\n"); | ||
| try parts.append(allocator, " var req = try client.request(."); | ||
| try parts.append(allocator, method); | ||
| try parts.append(allocator, ", uri, .{\n"); | ||
| try parts.append(allocator, | ||
| \\ .server_header_buffer = buf, | ||
| \\ }); | ||
| \\ defer req.deinit(); | ||
| \\ | ||
| \\ try req.send(); | ||
| ); | ||
| try parts.append(allocator, ", uri, .{});\n"); | ||
| try parts.append(allocator, " defer req.deinit();\n"); | ||
| if (has_request_body) { | ||
| try parts.append(allocator, "\n\n"); | ||
| try parts.append(allocator, " var str = std.ArrayList([]const u8){};\n"); | ||
| try parts.append(allocator, " defer str.deinit(allocator);\n\n"); | ||
| try parts.append(allocator, " try std.json.stringify(requestBody, .{}, str.writer());\n"); | ||
| try parts.append(allocator, " const body = try std.mem.join(allocator, \"\", str.items);\n"); | ||
| try parts.append(allocator, " defer allocator.free(body);\n\n"); | ||
| try parts.append(allocator, " req.transfer_encoding = .{ .content_length = body.len };\n"); | ||
| try parts.append(allocator, " try req.writeAll(body);\n\n"); | ||
| } else { | ||
| try parts.append(allocator, "\n"); | ||
| try parts.append(allocator, " var str: std.Io.Writer.Allocating = .init(allocator);\n"); | ||
| try parts.append(allocator, " defer str.deinit();\n\n"); | ||
| try parts.append(allocator, " try std.json.Stringify.value(requestBody, .{}, &str.writer);\n"); | ||
| try parts.append(allocator, " const body = str.written();\n\n"); | ||
| try parts.append(allocator, " try req.sendBodyComplete(body);\n"); | ||
| } else { | ||
| try parts.append(allocator, "\n try req.sendBodiless();\n"); | ||
| } |
Contributor
There was a problem hiding this comment.
Same incomplete request lifecycle as the v2.0 generator — receiveHead is not emitted.
This generator emits sendBodyComplete / sendBodiless and stops, which means generated v3.0 client methods will never read response headers or body and will silently ignore HTTP error statuses. See the detailed write-up on src/generators/v2.0/apigenerator.zig (lines 208–222) — the fix should be applied symmetrically here so v2.0/v3.0 generated output matches the reference flow in src/input_loader.zig:60–95.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/generators/v3.0/apigenerator.zig` around lines 191 - 205, The generated
v3.0 client stops after sendBodyComplete/sendBodiless and never reads the
response, so you must add the missing response lifecycle calls: after try
req.sendBodyComplete(...) / try req.sendBodiless(), call try req.receiveHead()
and inspect the returned head (e.g. head.statusCode) and then loop over
req.receiveBodyChunk / try req.receiveBodyComplete to collect the body (or use
the same helpers used in v2.0), propagate HTTP error statuses, and return the
assembled response; update the code that builds the method around the req
variable and client.request(...) to mirror the reference flow in
src/input_loader.zig (lines 60–95) so v3.0 emits req.receiveHead(), body-chunk
handling, and proper error handling just like the v2.0 generator.
| try generateSchema(allocator, parts, schema_name, schema_ptr.*); | ||
| }, | ||
| .reference => |_| {}, | ||
| .reference => {}, |
Contributor
There was a problem hiding this comment.
Top-level referenced schemas are silently dropped.
No-op handling for .reference means some components.schemas entries never become generated Zig types. Please resolve the reference (or emit a type alias) instead of skipping it.
As per coding guidelines, "Use unified converter architecture (src/generators/converters/) and unified generators (src/generators/unified/) instead of version-specific generators for code generation".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/generators/v3.0/modelgenerator.zig` at line 45, The .reference arm in
modelgenerator.zig currently no-ops (".reference => {}"), causing top-level
components.schemas references to be dropped; update the .reference case in the
switch handling within modelgenerator.zig to resolve the Reference object and
either generate a type alias or delegate to the unified converter/generator
pipeline (use the converters in src/generators/converters/ and unified
generators in src/generators/unified/) so that referenced schemas produce
emitted Zig types (i.e., look up the target schema by $ref, produce an alias
type name matching the reference key or forward to the unified generator to
generate the actual type).
8195545 to
fce97b7
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/models/v3.1/schema.zig (1)
135-137:⚠️ Potential issue | 🟠 MajorChange
exclusiveMaximumandexclusiveMinimumfrom?boolto?f64In OpenAPI 3.1 / JSON Schema 2020-12,
exclusiveMaximumandexclusiveMinimumare numeric bounds, not booleans. The current?booldeclaration will silently discard numeric values from real 3.1 specifications. Update both the struct fields and parsing logic:Fix
- exclusiveMaximum: ?bool = null, + exclusiveMaximum: ?f64 = null, minimum: ?f64 = null, - exclusiveMinimum: ?bool = null, + exclusiveMinimum: ?f64 = null, ... - .exclusiveMaximum = optionalBool(obj.get("exclusiveMaximum")), + .exclusiveMaximum = optionalFloat(obj.get("exclusiveMaximum")), .minimum = optionalFloat(obj.get("minimum")), - .exclusiveMinimum = optionalBool(obj.get("exclusiveMinimum")), + .exclusiveMinimum = optionalFloat(obj.get("exclusiveMinimum")),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v3.1/schema.zig` around lines 135 - 137, The struct fields exclusiveMaximum and exclusiveMinimum are declared as ?bool but should be ?f64 per OpenAPI 3.1/JSON Schema 2020-12; change their types to ?f64 in the Schema struct and update all parsing/deserialization logic that reads those properties (e.g., the JSON-to-Schema parser routines that currently treat those keys as booleans) to accept numeric values (parse as float64), store them into exclusiveMaximum/exclusiveMinimum, and adjust any validation/usage sites that assumed a boolean (including comparisons against minimum/maximum and presence checks) to handle optional floats correctly.
♻️ Duplicate comments (6)
src/generators/unified/api_generator.zig (5)
226-247:⚠️ Potential issue | 🟠 MajorMemory leak in
appendClientHeaderson error paths — the previously-claimed fix is not present in this code.After
auth_header = try std.fmt.allocPrint(...)at line 234, every subsequentheaders.append(...)(Authorization at 235, organization at 238, project at 241, default_headers loop at 244) can return an error. If any does,appendClientHeadersreturns the error and never returnsauth_header, so the caller'sdefer if (auth_header) |value| allocator.free(value);(line 205 inrequestRawand line 368 in generated operations) binds tonulland the allocation leaks.This is exactly the issue the prior review flagged and that was annotated as "Addressed in commit 8195545"; please verify against the current branch — the current text still allocates without an
errdefer.🛡️ Suggested fix inside the emitted helper
\\ var auth_header: ?[]u8 = null; \\ if (client.api_key.len > 0) { \\ auth_header = try std.fmt.allocPrint(allocator, "Bearer {s}", .{client.api_key}); + \\ errdefer if (auth_header) |v| allocator.free(v); \\ try headers.append(allocator, .{ .name = "Authorization", .value = auth_header.? }); \\ }As per coding guidelines: "Do not introduce memory leaks in any commit".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/generators/unified/api_generator.zig` around lines 226 - 247, The helper appendClientHeaders allocates auth_header via std.fmt.allocPrint but may fail later on headers.append calls and return early, leaking auth_header; update appendClientHeaders so that immediately after auth_header is assigned (in appendClientHeaders) you register an errdefer/cleanup that frees auth_header on error (and cancel it on success before return), or equivalently ensure every early-return path frees auth_header before returning; reference appendClientHeaders, auth_header, std.fmt.allocPrint, headers.append, and the callers requestRaw (which expects to free auth_header) to ensure the caller’s defer logic still works and no allocation is leaked.
296-302:⚠️ Potential issue | 🔴 CriticalFallback operation name still collides across HTTP methods on the same path.
When
operationIdis absent, the emitted identifier is@"operation" ++ path[1..]and themethodparameter is unused.GET /usersandPOST /userstherefore both producepub fn @"operationusers"(...), which fails to compile. Additionally,path[1..]is appended raw inside@"...", so a path containing"or\would corrupt the literal.A previous review flagged this and the thread was marked "Addressed in commit 8195545", but the code in this revision is unchanged. Routing the constructed name through
appendIdentifier(which already handles escaping) and includingmethodwill fix both issues:🐛 Suggested fix
if (operation.operationId) |op_id| { try self.appendIdentifier(op_id); } else { - try self.buffer.appendSlice(self.allocator, "@\"operation"); - try self.buffer.appendSlice(self.allocator, path[1..]); - try self.buffer.appendSlice(self.allocator, "\""); + var name_buf: std.ArrayList(u8) = .empty; + defer name_buf.deinit(self.allocator); + try name_buf.appendSlice(self.allocator, method); + try name_buf.appendSlice(self.allocator, "_"); + try name_buf.appendSlice(self.allocator, path[1..]); + try self.appendIdentifier(name_buf.items); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/generators/unified/api_generator.zig` around lines 296 - 302, When operation.operationId is missing, the code currently builds an unescaped literal with try self.buffer.appendSlice(..., "@\"operation"); try self.buffer.appendSlice(..., path[1..]); try self.buffer.appendSlice(..., "\"") which both ignores the HTTP method (causing GET/POST name collisions) and fails to escape `"` or `\`; instead construct a fallback name that includes the method and the path and pass that string to self.appendIdentifier so escaping/identifier rules are applied (e.g., build a small temp string like "operation" + method + path[1..] and call try self.appendIdentifier(temp) rather than appending raw slices to buffer); update the branch that checks operation.operationId to use appendIdentifier for the fallback case and stop using raw appendSlice for path fragments.
422-444:⚠️ Potential issue | 🟡 Minor
parameter.namestill embedded raw in a generated string literal for query params.Lines 429 and 438 do
appendSlice("\"") + appendSlice(parameter.name) + appendSlice("\", ..."), producing:try appendQueryParam(&uri_buf.writer, &first_query, "<name>", value);If
<name>contains",\, or a control character, the emitted source is malformed. The previous review on this exact concern was marked addressed but the raw concatenation is still here. Reuse the same string-literal escape helper recommended formodel_generator.zig::generateJsonStringifyrather than re-inventing it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/generators/unified/api_generator.zig` around lines 422 - 444, The generated code embeds parameter.name raw into a quoted literal for appendQueryParam, which can produce invalid Zig source if the name contains quotes, backslashes, or control chars; replace the raw appendSlice(parameter.name) used at the quoted positions (the occurrences around try appendQueryParam(..., "\" + parameter.name + "\", ...)) with the existing string-literal escape helper used by model_generator.zig::generateJsonStringify (or extract that helper into a shared function) so you emit a properly escaped literal (e.g., call escapeStringLiteral(self, parameter.name) or generateJsonStringify-style routine) when writing the quoted query name; keep other uses of appendSlice(parameter.name) that are not inside generated string literals unchanged and ensure the helper returns/prints the escaped contents including surrounding quotes.
538-561:⚠️ Potential issue | 🟠 MajorArray element types are still erased in
appendZigTypeFromSchemaType, so the API and model layers disagree.
appendZigTypeFromSchema(lines 538-550) delegates the array case toappendZigTypeFromSchemaType, which unconditionally emits[]const std.json.Value(line 558) and ignoresschema.items. Meanwhilemodel_generator.zig::appendZigType/appendArrayItemType(lines 214-266) correctly recurse onitemsand emit[]const T.Concrete impact: a
requestBodywhose schema isarray of $ref/Foois generated as a parameter of type[]const std.json.Valuein the API client, while the corresponding model field would be[]const Foo— making the two halves mutually incompatible. Same for return types whose 2xx response is an array of refs.Make
appendZigTypeFromSchemahandle.arrayitself by recursing intoschema.items, and reserveappendZigTypeFromSchemaTypefor sites where only aSchemaTypeis available (e.g.,appendZigQueryTypeFromSchemaalready does this correctly for query params, which are scalar by construction).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/generators/unified/api_generator.zig` around lines 538 - 561, appendZigTypeFromSchema must handle array schemas by recursing into schema.items so element types (refs or primitives) are preserved: change appendZigTypeFromSchema to detect when schema.type == .array, call into the same path you use for other schemas (e.g., if items.ref -> appendIdentifier, if items.type -> call appendZigTypeFromSchemaType for the item and then prefix with "[]const "), and only fall back to "std.json.Value" for unknown items; correspondingly, remove/avoid emitting concrete array element types from appendZigTypeFromSchemaType (leave it for scalar SchemaType cases like query params) so appendZigTypeFromSchema is the authoritative handler for arrays and keeps API types consistent with model_generator.zig::appendZigType/appendArrayItemType.
340-361:⚠️ Potential issue | 🔴 CriticalHeader and cookie parameters are still silently dropped.
The body of
generateFunctionBodyskips parameters whose location is neitherpath,body, norqueryand emits_ = name;. For specs that require header parameters (e.g.X-Request-ID,Authorizationoverrides) or cookie parameters, the generated client takes the value as a function argument and then immediately discards it, producing an outwardly-correct signature with broken behavior. The previous review flagged this; please apply header/cookie handling here.🛡️ Suggested fix
- if (parameter.location != .path and parameter.location != .body and parameter.location != .query) { - try self.buffer.appendSlice(self.allocator, " _ = "); - try self.appendIdentifier(parameter.name); - try self.buffer.appendSlice(self.allocator, ";\n"); - } + switch (parameter.location) { + .header => { + try self.buffer.appendSlice(self.allocator, " try headers.append(allocator, .{ .name = \""); + try self.buffer.appendSlice(self.allocator, parameter.name); // TODO: escape + try self.buffer.appendSlice(self.allocator, "\", .value = "); + try self.appendIdentifier(parameter.name); + try self.buffer.appendSlice(self.allocator, " });\n"); + }, + .cookie => { /* accumulate name=value into a single Cookie header */ }, + else => {}, + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/generators/unified/api_generator.zig` around lines 340 - 361, The function generateFunctionBody currently drops parameters whose location is not .path/.body/.query by emitting "_ = <name>;", so update generateFunctionBody to handle .header and .cookie parameters instead of discarding them: locate the loop over parameters (the for (parameters) |parameter| block) and replace the discard branch for header and cookie with logic that adds the parameter to the outgoing request (for .header, add a request header using the parameter.name as the header key and the argument value as the header value; for .cookie, accumulate cookie name/value pairs and set them on the Cookie header or cookie container used by the client). Preserve existing behavior for .path/.body/.query; remove the "_ = <name>;" discard lines and ensure proper encoding/escaping of values when inserting into headers.src/generators/unified/model_generator.zig (1)
166-212:⚠️ Potential issue | 🟡 Minor
field_nameis embedded raw into a generated"..."string literal.Lines 178-180 and 188-190 do
appendSlice("\"") + appendSlice(field_name) + appendSlice("\")")to emittry jw.objectField("<name>");. If a property name in the input spec contains",\, or a control character, the generated source will be malformed (and the JSON key will also be wrong). This is the same class of issue the previous review flagged forparameter.namein the API generator — please reuse the same string-literal escape helper here.🛡️ Suggested approach
Extract a shared
appendStringLiteralBody(name)helper (mirroring the escape branches insideappendIdentifier) and call it instead of the bareappendSlice(field_name)at both call sites ingenerateJsonStringify.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/generators/unified/model_generator.zig` around lines 166 - 212, generateJsonStringify currently embeds field_name directly into emitted string literals (e.g., the try jw.objectField("...") calls), which breaks when names contain quotes, backslashes, or control chars; add and reuse a string-literal escaping helper (e.g., appendStringLiteralBody(name): fn on UnifiedModelGenerator) that mirrors the escaping logic used by appendIdentifier, and call that helper instead of appendSlice(field_name) in both places that emit the objectField string literal; ensure the helper writes only the inner literal body (not surrounding quotes) and is used wherever field_name is placed inside a quoted string in generateJsonStringify so generated Zig source is correctly escaped.
🧹 Nitpick comments (2)
src/models/v3.1/schema.zig (1)
264-270: Inconsistent boolean parsing — applyoptionalBoolto remaining bool fields.
nullable,readOnly,writeOnly, anddeprecatedstill dereferenceval.booldirectly, so any spec where these are emitted as e.g."true"/"false"strings (or a wrong type) will panic at runtime, while the constraint fields you just hardened won't. Same applies toXML.parseFromJson(lines 46–47). Routing them throughoptionalBoolkeeps behavior consistent with the rest of this PR.Proposed fix
- .nullable = if (obj.get("nullable")) |val| val.bool else null, + .nullable = optionalBool(obj.get("nullable")), .discriminator = if (obj.get("discriminator")) |val| try Discriminator.parseFromJson(allocator, val) else null, - .readOnly = if (obj.get("readOnly")) |val| val.bool else null, - .writeOnly = if (obj.get("writeOnly")) |val| val.bool else null, + .readOnly = optionalBool(obj.get("readOnly")), + .writeOnly = optionalBool(obj.get("writeOnly")), @@ - .deprecated = if (obj.get("deprecated")) |val| val.bool else null, + .deprecated = optionalBool(obj.get("deprecated")),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v3.1/schema.zig` around lines 264 - 270, The boolean fields are being read directly via val.bool which can panic for non-boolean JSON; update the parsing of .nullable, .readOnly, .writeOnly, and .deprecated to use the existing optionalBool helper instead of val.bool, and likewise route XML parsing through XML.parseFromJson's use of optionalBool (check XML.parseFromJson and replace direct val.bool accesses with optionalBool) so all boolean fields use the same safe optionalBool parsing logic (refer to the field names .nullable, .readOnly, .writeOnly, .deprecated and the XML.parseFromJson function to locate the changes).src/generators/unified/model_generator.zig (1)
5-41: Identifier helpers duplicated verbatim withapi_generator.zig.
isIdentStart,isIdentContinue,isReservedIdent,isBareIdentifierand the reserved-keyword list are byte-for-byte duplicated insrc/generators/unified/api_generator.zig(lines 8-39). Any future divergence (new Zig keyword, escape rule change) would silently produce inconsistent output between models and API client. Consider extracting into a shared helper module, e.g.src/generators/unified/zig_idents.zig, used by both generators (andappendIdentifiertoo).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/generators/unified/model_generator.zig` around lines 5 - 41, The identifier helper functions (isIdentStart, isIdentContinue, isReservedIdent, isBareIdentifier) and the reserved-keyword list are duplicated and should be extracted into a shared module (suggested name: zig_idents.zig); create that module containing the reserved list and the functions, export them, and update both model_generator.zig and api_generator.zig to import and call those shared symbols (also move/replace any appendIdentifier usage to reference the shared helper if applicable) so both generators use the single canonical implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/generators/unified/model_generator.zig`:
- Around line 38-41: Replace the hardcoded OpenAI schema-name checks in
isExtensibleRequest and the related logic that injects reasoning_details and
extra_body/jsonStringify with a configurable switch: add a generator option
(e.g. openaiExtensions: bool) or check a vendor-extension annotation on the
schema, then only apply the special-case transformations when that
flag/annotation is present; update the isExtensibleRequest function and the code
paths that reference ChatCompletionRequestAssistantMessage, CreateResponse,
CreateChatCompletionRequest, reasoning_details, extra_body, jsonStringify, and
emit_null_optional_fields to consult this new flag/annotation instead of
comparing literal names so non-OpenAI specs remain untouched and OpenAI-specific
behavior is opt-in.
---
Outside diff comments:
In `@src/models/v3.1/schema.zig`:
- Around line 135-137: The struct fields exclusiveMaximum and exclusiveMinimum
are declared as ?bool but should be ?f64 per OpenAPI 3.1/JSON Schema 2020-12;
change their types to ?f64 in the Schema struct and update all
parsing/deserialization logic that reads those properties (e.g., the
JSON-to-Schema parser routines that currently treat those keys as booleans) to
accept numeric values (parse as float64), store them into
exclusiveMaximum/exclusiveMinimum, and adjust any validation/usage sites that
assumed a boolean (including comparisons against minimum/maximum and presence
checks) to handle optional floats correctly.
---
Duplicate comments:
In `@src/generators/unified/api_generator.zig`:
- Around line 226-247: The helper appendClientHeaders allocates auth_header via
std.fmt.allocPrint but may fail later on headers.append calls and return early,
leaking auth_header; update appendClientHeaders so that immediately after
auth_header is assigned (in appendClientHeaders) you register an
errdefer/cleanup that frees auth_header on error (and cancel it on success
before return), or equivalently ensure every early-return path frees auth_header
before returning; reference appendClientHeaders, auth_header,
std.fmt.allocPrint, headers.append, and the callers requestRaw (which expects to
free auth_header) to ensure the caller’s defer logic still works and no
allocation is leaked.
- Around line 296-302: When operation.operationId is missing, the code currently
builds an unescaped literal with try self.buffer.appendSlice(...,
"@\"operation"); try self.buffer.appendSlice(..., path[1..]); try
self.buffer.appendSlice(..., "\"") which both ignores the HTTP method (causing
GET/POST name collisions) and fails to escape `"` or `\`; instead construct a
fallback name that includes the method and the path and pass that string to
self.appendIdentifier so escaping/identifier rules are applied (e.g., build a
small temp string like "operation" + method + path[1..] and call try
self.appendIdentifier(temp) rather than appending raw slices to buffer); update
the branch that checks operation.operationId to use appendIdentifier for the
fallback case and stop using raw appendSlice for path fragments.
- Around line 422-444: The generated code embeds parameter.name raw into a
quoted literal for appendQueryParam, which can produce invalid Zig source if the
name contains quotes, backslashes, or control chars; replace the raw
appendSlice(parameter.name) used at the quoted positions (the occurrences around
try appendQueryParam(..., "\" + parameter.name + "\", ...)) with the existing
string-literal escape helper used by model_generator.zig::generateJsonStringify
(or extract that helper into a shared function) so you emit a properly escaped
literal (e.g., call escapeStringLiteral(self, parameter.name) or
generateJsonStringify-style routine) when writing the quoted query name; keep
other uses of appendSlice(parameter.name) that are not inside generated string
literals unchanged and ensure the helper returns/prints the escaped contents
including surrounding quotes.
- Around line 538-561: appendZigTypeFromSchema must handle array schemas by
recursing into schema.items so element types (refs or primitives) are preserved:
change appendZigTypeFromSchema to detect when schema.type == .array, call into
the same path you use for other schemas (e.g., if items.ref -> appendIdentifier,
if items.type -> call appendZigTypeFromSchemaType for the item and then prefix
with "[]const "), and only fall back to "std.json.Value" for unknown items;
correspondingly, remove/avoid emitting concrete array element types from
appendZigTypeFromSchemaType (leave it for scalar SchemaType cases like query
params) so appendZigTypeFromSchema is the authoritative handler for arrays and
keeps API types consistent with
model_generator.zig::appendZigType/appendArrayItemType.
- Around line 340-361: The function generateFunctionBody currently drops
parameters whose location is not .path/.body/.query by emitting "_ = <name>;",
so update generateFunctionBody to handle .header and .cookie parameters instead
of discarding them: locate the loop over parameters (the for (parameters)
|parameter| block) and replace the discard branch for header and cookie with
logic that adds the parameter to the outgoing request (for .header, add a
request header using the parameter.name as the header key and the argument value
as the header value; for .cookie, accumulate cookie name/value pairs and set
them on the Cookie header or cookie container used by the client). Preserve
existing behavior for .path/.body/.query; remove the "_ = <name>;" discard lines
and ensure proper encoding/escaping of values when inserting into headers.
In `@src/generators/unified/model_generator.zig`:
- Around line 166-212: generateJsonStringify currently embeds field_name
directly into emitted string literals (e.g., the try jw.objectField("...")
calls), which breaks when names contain quotes, backslashes, or control chars;
add and reuse a string-literal escaping helper (e.g.,
appendStringLiteralBody(name): fn on UnifiedModelGenerator) that mirrors the
escaping logic used by appendIdentifier, and call that helper instead of
appendSlice(field_name) in both places that emit the objectField string literal;
ensure the helper writes only the inner literal body (not surrounding quotes)
and is used wherever field_name is placed inside a quoted string in
generateJsonStringify so generated Zig source is correctly escaped.
---
Nitpick comments:
In `@src/generators/unified/model_generator.zig`:
- Around line 5-41: The identifier helper functions (isIdentStart,
isIdentContinue, isReservedIdent, isBareIdentifier) and the reserved-keyword
list are duplicated and should be extracted into a shared module (suggested
name: zig_idents.zig); create that module containing the reserved list and the
functions, export them, and update both model_generator.zig and
api_generator.zig to import and call those shared symbols (also move/replace any
appendIdentifier usage to reference the shared helper if applicable) so both
generators use the single canonical implementation.
In `@src/models/v3.1/schema.zig`:
- Around line 264-270: The boolean fields are being read directly via val.bool
which can panic for non-boolean JSON; update the parsing of .nullable,
.readOnly, .writeOnly, and .deprecated to use the existing optionalBool helper
instead of val.bool, and likewise route XML parsing through XML.parseFromJson's
use of optionalBool (check XML.parseFromJson and replace direct val.bool
accesses with optionalBool) so all boolean fields use the same safe optionalBool
parsing logic (refer to the field names .nullable, .readOnly, .writeOnly,
.deprecated and the XML.parseFromJson function to locate the changes).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 25820d74-3e40-4330-905d-1a214a64eb42
⛔ Files ignored due to path filters (5)
generated/generated_v2.zigis excluded by!**/generated/**generated/generated_v3.zigis excluded by!**/generated/**generated/generated_v31.zigis excluded by!**/generated/**generated/generated_v32.zigis excluded by!**/generated/**generated/main.zigis excluded by!**/generated/**
📒 Files selected for processing (6)
src/generators/converters/openapi31_converter.zigsrc/generators/converters/openapi32_converter.zigsrc/generators/converters/openapi_converter.zigsrc/generators/unified/api_generator.zigsrc/generators/unified/model_generator.zigsrc/models/v3.1/schema.zig
🚧 Files skipped from review as they are similar to previous changes (1)
- src/generators/converters/openapi31_converter.zig
Comment on lines
+38
to
+41
| fn isExtensibleRequest(name: []const u8) bool { | ||
| return std.mem.eql(u8, name, "CreateResponse") or | ||
| std.mem.eql(u8, name, "CreateChatCompletionRequest"); | ||
| } |
Contributor
There was a problem hiding this comment.
Hardcoded OpenAI-specific schema names defeat the generic-OpenAPI premise.
Three names — ChatCompletionRequestAssistantMessage (line 117), CreateResponse, CreateChatCompletionRequest (lines 39-40) — are literally compared as strings. For any non-OpenAI spec these branches never fire, and for any future OpenAI schema rename they silently stop firing. Two specific consequences:
- The
reasoning_details: ?std.json.Value = nullinjection only happens for one named struct. - The
extra_bodyextension and the correspondingjsonStringifyare only emitted for two named requests, soemit_null_optional_fields = false(used in the API generator) is the only mechanism keeping other generated requests serialization-friendly.
If these are genuinely needed for the OpenAI workflow, please at minimum gate them behind a CLI flag (e.g. --openai-extensions) or a vendor-extension annotation in the input spec, rather than baking schema names into the unified generator.
Also applies to: 117-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/generators/unified/model_generator.zig` around lines 38 - 41, Replace the
hardcoded OpenAI schema-name checks in isExtensibleRequest and the related logic
that injects reasoning_details and extra_body/jsonStringify with a configurable
switch: add a generator option (e.g. openaiExtensions: bool) or check a
vendor-extension annotation on the schema, then only apply the special-case
transformations when that flag/annotation is present; update the
isExtensibleRequest function and the code paths that reference
ChatCompletionRequestAssistantMessage, CreateResponse,
CreateChatCompletionRequest, reasoning_details, extra_body, jsonStringify, and
emit_null_optional_fields to consult this new flag/annotation instead of
comparing literal names so non-OpenAI specs remain untouched and OpenAI-specific
behavior is opt-in.
e89432d to
f812fa0
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/generators/unified/api_generator.zig (1)
1084-1118:⚠️ Potential issue | 🟠 MajorPath placeholder substitution corrupts paths when a parameter name appears as a substring of a literal path segment.
std.mem.replace(u8, new_path, parameter.name, param_type, output)replaces the parameter name as a raw substring rather than the{paramName}placeholder. Any literal path segment that contains a parameter name as a substring is silently rewritten:
/items/{id}/identifierwithid: integer→/items/{d}/dentifier/pets/{petId}/petIdHistorywithpetId: integer→/pets/{d}/dIdHistoryThis produces wrong runtime URLs (and, when subsequently embedded into the format-string literal at lines 1108-1110, is also a source-corruption hazard if the path contains
"or\). The replacement should target the placeholder including braces.🐛 Suggested fix
- const param = parameter.name; + const placeholder = try std.fmt.allocPrint(self.allocator, "{{{s}}}", .{parameter.name}); + try allocated_paths.append(self.allocator, placeholder); const path_type = if (parameter.schema) |schema| schema.type orelse .string else parameter.type orelse .string; - const param_type = switch (path_type) { + const fmt_spec = switch (path_type) { .string => "s", .integer => "d", .number => "d", else => "any", }; - const size = std.mem.replacementSize(u8, new_path, param, param_type); + const replacement = try std.fmt.allocPrint(self.allocator, "{{{s}}}", .{fmt_spec}); + try allocated_paths.append(self.allocator, replacement); + const size = std.mem.replacementSize(u8, new_path, placeholder, replacement); const output = try self.allocator.alloc(u8, size); try allocated_paths.append(self.allocator, output); - _ = std.mem.replace(u8, new_path, param, param_type, output); + _ = std.mem.replace(u8, new_path, placeholder, replacement, output); new_path = output;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/generators/unified/api_generator.zig` around lines 1084 - 1118, The replace logic is matching raw parameter.name and corrupts literal segments; build a placeholder string that includes braces (e.g. placeholder = "{" ++ parameter.name ++ "}") and use that placeholder when calling std.mem.replacementSize and std.mem.replace (replace occurrences of parameter.name with placeholder in the calls that reference new_path, output, replacementSize, and replace). Also ensure the final new_path content is safely embedded into the format-string emitted to the buffer (escape `"` and `\` or use a safe formatting API) when calling appendSlice/print so the generated string literal can’t be broken by path characters.
🧹 Nitpick comments (1)
src/generators/unified/api_generator.zig (1)
5-39: Duplicated identifier/keyword helpers across generators.
isIdentStart,isIdentContinue,isReservedIdent,isBareIdentifier, andappendIdentifierare byte-identical (and inappendIdentifier's case nearly so) to those insrc/generators/unified/model_generator.zig. Extract to a small shared helper module (e.g.src/generators/unified/identifiers.zig) so the reserved-keyword list stays consistent between model and API code generation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/generators/unified/api_generator.zig` around lines 5 - 39, The helper functions isIdentStart, isIdentContinue, isReservedIdent, isBareIdentifier (and appendIdentifier if present) are duplicated; extract them into a new shared module (e.g., identifiers.zig) and have api_generator.zig and model_generator.zig import and call the centralized functions instead of defining their own copies; move the reserved keyword array into that module so both generators use the single source of truth, update references in api_generator.zig to call the imported functions (isIdentStart, isIdentContinue, isReservedIdent, isBareIdentifier, appendIdentifier) and remove the local definitions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/generators/unified/api_generator.zig`:
- Around line 326-331: parseRawResponse currently takes RawResponse by value but
transfers ownership of RawResponse.body into the returned ApiResult causing
potential double-free if the caller still calls RawResponse.deinit(); update
parseRawResponse to either take a pointer to RawResponse and clear/sentinel the
source body after moving it (e.g., set raw_ptr.body = null or similar) or
clearly document consumption by adding a doc comment on parseRawResponse stating
"consumes raw; do not call raw.deinit() after this returns"; refer to
parseRawResponse, RawResponse.body, RawResponse.deinit, and related helpers
getJsonResult/postJsonResult when making the change so callers are adjusted
accordingly.
In `@src/generators/unified/model_generator.zig`:
- Around line 196-211: The generated jsonStringify currently appends extra_body
entries unconditionally which can emit duplicate keys; update the serialization
logic in model_generator.zig so that when writing declared struct fields you
record each emitted field name (e.g., into a small HashSet or zig.StringHashMap)
and then, in the extra_body iteration (the block that calls
jw.objectField(entry.key_ptr.*) / jw.write(entry.value_ptr.*)), skip any entry
whose key is present in that set; reference the symbols extra_body,
jsonStringify (or the generator routine emitting this snippet), jw.objectField
and jw.write when locating where to add the set population and the membership
check.
---
Outside diff comments:
In `@src/generators/unified/api_generator.zig`:
- Around line 1084-1118: The replace logic is matching raw parameter.name and
corrupts literal segments; build a placeholder string that includes braces (e.g.
placeholder = "{" ++ parameter.name ++ "}") and use that placeholder when
calling std.mem.replacementSize and std.mem.replace (replace occurrences of
parameter.name with placeholder in the calls that reference new_path, output,
replacementSize, and replace). Also ensure the final new_path content is safely
embedded into the format-string emitted to the buffer (escape `"` and `\` or use
a safe formatting API) when calling appendSlice/print so the generated string
literal can’t be broken by path characters.
---
Nitpick comments:
In `@src/generators/unified/api_generator.zig`:
- Around line 5-39: The helper functions isIdentStart, isIdentContinue,
isReservedIdent, isBareIdentifier (and appendIdentifier if present) are
duplicated; extract them into a new shared module (e.g., identifiers.zig) and
have api_generator.zig and model_generator.zig import and call the centralized
functions instead of defining their own copies; move the reserved keyword array
into that module so both generators use the single source of truth, update
references in api_generator.zig to call the imported functions (isIdentStart,
isIdentContinue, isReservedIdent, isBareIdentifier, appendIdentifier) and remove
the local definitions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eea1a7da-16f9-4994-b2e0-40a41a626ad8
⛔ Files ignored due to path filters (6)
generated/compile_generated.zigis excluded by!**/generated/**generated/generated_v2.zigis excluded by!**/generated/**generated/generated_v3.zigis excluded by!**/generated/**generated/generated_v31.zigis excluded by!**/generated/**generated/generated_v32.zigis excluded by!**/generated/**generated/main.zigis excluded by!**/generated/**
📒 Files selected for processing (10)
docs/openai-generation-issues.mdsrc/cli.zigsrc/generators/converters/openapi31_converter.zigsrc/generators/converters/openapi32_converter.zigsrc/generators/converters/openapi_converter.zigsrc/generators/unified/api_generator.zigsrc/generators/unified/model_generator.zigsrc/models/v3.1/schema.zigsrc/tests.zigsrc/tests/resource_wrapper_tests.zig
✅ Files skipped from review due to trivial changes (3)
- src/tests.zig
- src/generators/converters/openapi_converter.zig
- src/models/v3.1/schema.zig
🚧 Files skipped from review as they are similar to previous changes (2)
- src/generators/converters/openapi32_converter.zig
- src/generators/converters/openapi31_converter.zig
Comment on lines
+196
to
+211
| try self.buffer.appendSlice(self.allocator, | ||
| \\ | ||
| \\ if (self.extra_body) |extra| { | ||
| \\ if (extra == .object) { | ||
| \\ var iterator = extra.object.iterator(); | ||
| \\ while (iterator.next()) |entry| { | ||
| \\ try jw.objectField(entry.key_ptr.*); | ||
| \\ try jw.write(entry.value_ptr.*); | ||
| \\ } | ||
| \\ } | ||
| \\ } | ||
| \\ | ||
| \\ try jw.endObject(); | ||
| \\ } | ||
| \\ | ||
| ); |
Contributor
There was a problem hiding this comment.
extra_body flattening can emit duplicate JSON keys.
When extra_body contains a key that already matches a serialized struct field (the very common OpenRouter override pattern, e.g. extra_body = {"model": "..."}), the generated jsonStringify will emit that key twice. The output is technically RFC 8259-tolerant but most consumers take only one value (often the first), which is the opposite of caller intent.
Consider tracking the field names already written and skipping duplicates from extra_body, or document clearly that extra_body keys must not overlap with declared fields.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/generators/unified/model_generator.zig` around lines 196 - 211, The
generated jsonStringify currently appends extra_body entries unconditionally
which can emit duplicate keys; update the serialization logic in
model_generator.zig so that when writing declared struct fields you record each
emitted field name (e.g., into a small HashSet or zig.StringHashMap) and then,
in the extra_body iteration (the block that calls
jw.objectField(entry.key_ptr.*) / jw.write(entry.value_ptr.*)), skip any entry
whose key is present in that set; reference the symbols extra_body,
jsonStringify (or the generator routine emitting this snippet), jw.objectField
and jw.write when locating where to add the set population and the membership
check.
f812fa0 to
5dc1078
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/models/v3.1/schema.zig (1)
134-137:⚠️ Potential issue | 🟠 Major
exclusiveMaximumandexclusiveMinimummust be numeric in OpenAPI 3.1, not bool.OpenAPI 3.1 follows JSON Schema 2020-12, where these fields are numbers (the exclusive bound value itself), not booleans as in 3.0. With the current type
?boolandoptionalBool()helper, real-world 3.1 specs emitting"exclusiveMinimum": 0silently drop the value (sinceoptionalBoolreturnsnullfor non-bool inputs), defeating the PR's stated goal of parsing real OpenAPI 3.1 specifications.Fix
maximum: ?f64 = null, - exclusiveMaximum: ?bool = null, + exclusiveMaximum: ?f64 = null, minimum: ?f64 = null, - exclusiveMinimum: ?bool = null, + exclusiveMinimum: ?f64 = null,- .exclusiveMaximum = optionalBool(obj.get("exclusiveMaximum")), + .exclusiveMaximum = optionalFloat(obj.get("exclusiveMaximum")), .minimum = optionalFloat(obj.get("minimum")), - .exclusiveMinimum = optionalBool(obj.get("exclusiveMinimum")), + .exclusiveMinimum = optionalFloat(obj.get("exclusiveMinimum")),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v3.1/schema.zig` around lines 134 - 137, The exclusiveMaximum and exclusiveMinimum fields are incorrectly typed as ?bool and parsed with optionalBool(); change their types to ?f64 (numeric optional) in the schema definition (alongside maximum and minimum) and switch parsing/serialization to use the numeric optional helper (e.g., optionalNumber()) instead of optionalBool(); update any code paths that reference exclusiveMaximum/exclusiveMinimum (and the optionalBool() usage in this context) so numeric values like 0 are preserved for OpenAPI 3.1 / JSON Schema 2020-12 compatibility.
🧹 Nitpick comments (5)
docs/openai-generation-issues.md (2)
394-397: Optional: Vary sentence beginnings for better readability.Three consecutive bullet points start with "Add". Consider varying the phrasing for better flow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openai-generation-issues.md` around lines 394 - 397, The three consecutive bullets all start with "Add", which is repetitive; rephrase them for variety by turning one or two into alternative verbs or sentence structures—e.g., "Support extra_body flattening", "Introduce reasoning_details extension support", "Implement an initial streaming/SSE runtime", and "Begin multipart support or mark unsupported endpoints as raw"—update the lines containing the phrases "extra_body flattening", "reasoning_details extension support", "first streaming/SSE runtime", and "multipart support or mark unsupported endpoints raw" accordingly.
58-60: Optional: Vary sentence beginnings for better readability.Three consecutive sentences start with "Generated". Consider rephrasing for variety, e.g.:
- Generated endpoint response parsing uses `.ignore_unknown_fields = true`. -- Generated runtime exposes `RawResponse`, `requestRaw`, `getRaw`, and `postJsonRaw` so callers can inspect status and body. -- Generated runtime includes a bounded dynamic SSE parser: +- The runtime exposes `RawResponse`, `requestRaw`, `getRaw`, and `postJsonRaw` so callers can inspect status and body. +- A bounded dynamic SSE parser is included with:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openai-generation-issues.md` around lines 58 - 60, The three bullets all begin with the word "Generated", which is monotonous; reword them to vary sentence openings while preserving meaning—e.g., start the first with "The generated runtime exposes ..." or "Runtime exposes ..." for RawResponse / requestRaw / getRaw / postJsonRaw, change the second to "It also includes a bounded dynamic SSE parser" and the third to "Supported line endings: LF and CRLF" (or similar) so each bullet uses a different lead phrase but still references the same symbols (RawResponse, requestRaw, getRaw, postJsonRaw, SSE parser, LF, CRLF).src/tests/resource_wrapper_tests.zig (1)
16-43: Drop the unusedmethodparameter fromop.
optakesmethod: []const u8but immediately discards it (_ = method;). Either remove the parameter or use it (e.g., to set HTTP-method-specific operation defaults). Minor cleanup.♻️ Suggested change
-fn op(allocator: std.mem.Allocator, operation_id: []const u8, method: []const u8, has_body: bool, has_path_param: bool, has_response: bool) !common.Operation { +fn op(allocator: std.mem.Allocator, operation_id: []const u8, has_body: bool, has_path_param: bool, has_response: bool) !common.Operation { var params = std.ArrayList(common.Parameter).empty; errdefer params.deinit(allocator); ... - _ = method; return .{And update call sites in
buildFixtureaccordingly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tests/resource_wrapper_tests.zig` around lines 16 - 43, Remove the unused method parameter from the op function signature and all its call sites (e.g., in buildFixture) so op only accepts (allocator, operation_id, has_body, has_path_param, has_response); update references to the symbol op and any callers to stop passing the method argument (or alternatively, if you prefer to keep method, wire it into op's returned operation fields instead). Ensure you update the op declaration and every invocation site in tests/helpers (such as buildFixture) to match the new parameter list to fix the unused-variable discard (_ = method).src/cli.zig (1)
15-15: Defaultingresource_wrappersto.pathsis a behavior change for existing users.Anyone upgrading and re-running
openapi2zig generatewithout the new flag will now get apub const resources = struct { ... }namespace plus top-level aliases (pub const <segment> = resources.<segment>;) appended to every generated file. The aliases can also conflict with generated model type names (the code has a small block list —organization,project,value— but it's not exhaustive). Consider defaulting to.nonefor backward compatibility and letting users opt in via--resource-wrappers paths.♻️ Suggested change
pub const CliArgs = struct { input_path: []const u8, output_path: ?[]const u8 = null, base_url: ?[]const u8 = null, - resource_wrappers: ResourceWrapperMode = .paths, + resource_wrappers: ResourceWrapperMode = .none, }; ... - var resource_wrappers: ResourceWrapperMode = .paths; + var resource_wrappers: ResourceWrapperMode = .none;Then update
printUsage()to reflect(default: none).Also applies to: 38-38
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli.zig` at line 15, The default value for the CLI option resource_wrappers (ResourceWrapperMode) is set to .paths which changes behavior for existing users; change the default to .none to preserve backward compatibility, update any code that references resource_wrappers: ResourceWrapperMode = .paths to use .none instead, and update the usage text in printUsage() to indicate "(default: none)"; ensure the CLI flag parsing still accepts --resource-wrappers paths so users can opt in to .paths behavior when desired.src/generators/converters/openapi32_converter.zig (1)
339-353: Fallback iterator overStringHashMapcontent has non-deterministic order.When
application/jsonis absent and multiple content types exist (e.g.,text/xml+application/x-yaml),content.iterator().next()selects an unspecified entry, causing generated code to vary run-to-run for the same spec. This pattern appears in:
convertRequestBody(lines 342–353)convertResponse(lines 431–441)openapi31_converter.zig(lines 499 and 588)openapi_converter.zig(lines 333 and 422)In practice this is benign because most specs either have a single content type or prefer JSON, but deterministic output can be achieved by prioritizing other common formats (e.g., XML) or sorting available keys before selecting the fallback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/generators/converters/openapi32_converter.zig` around lines 339 - 353, The fallback that uses content.iterator().next() is non-deterministic; update convertRequestBody and convertResponse (and analogous logic in openapi31_converter.zig and openapi_converter.zig) to deterministically pick a media type: after checking "application/json", build a list of available content keys, then search them in a preferred-order list (e.g., "application/xml", "text/xml", "application/x-yaml", "text/yaml") and use the first match; if none match, sort the keys (lexicographically) and pick the first entry so output is stable across runs; replace the iterator().next() usage with this deterministic selection and then call convertSchemaOrReference on the chosen media type's schema.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/openai-generation-issues.md`:
- Around line 159-180: The remaining-issues entry wrongly claims lack of
reasoning_details support; either remove this entry or update it to specify
exactly which structs still miss support (e.g., identify which Request structs
and which generated assistant message structs do not preserve reasoning_details
via the explicit reasoning_details field or the extra_body/extra fields
mechanism). Search for the symbol reasoning_details and inspect the generator
that emits the "generated assistant message structs" and the Request struct
definitions to list any missing types, then update the doc to name those structs
or drop the issue if none are missing.
- Around line 210-230: The issue incorrectly claims streaming runtime is
missing; update the issue text to reflect that a bounded SSE parser and
callback-based streaming APIs exist by referencing the implemented
parseSseReader function and the generated streamChatCompletion and
streamResponse helpers (and their callbacks) in the generator, and change the
remaining work to only cover "typed stream event parsing" (or adding richer
typed event parsing on top of parseSseReader) so the issue no longer states
streaming is wholly unimplemented.
- Around line 181-209: The docs entry "4. No `extra_body` flattening" is out of
date — update the "Remaining issues" section to reflect that `extra_body` is
already flattened: reference the implemented symbols `extra_body`,
`CreateChatCompletionRequest`, and `CreateResponse`, and the implementation in
src/generators/unified/model_generator.zig where the extra_body field and
flattening logic are added; either remove the issue entirely or edit the text to
clarify what, if anything, remains missing (e.g., specific provider tests) and
point to the exact implementation locations for verification.
- Around line 112-158: The doc entry claiming parsing fails on unknown provider
fields is outdated—confirm that the generator already emits
std.json.parseFromSlice(..., .{ .ignore_unknown_fields = true }) (see
api_generator.zig where CreateChatCompletionResponse parsing is generated) and
either remove this item from the "Remaining issues" list or edit the doc to
state the specific exceptional scenario that still fails (if any), e.g.,
describe the exact endpoint or streaming case that does not get
.ignore_unknown_fields, otherwise mark the issue resolved.
In `@src/generators/converters/openapi31_converter.zig`:
- Around line 183-187: convertResolvedSchemaReference currently follows $ref
into convertSchemaOrReference which can re-enter via convertSchema ->
convertAllOfSchema and lead to infinite recursion for cyclic allOf references;
add a visited-set (e.g., a HashMap or Set) of resolved component names stored on
OpenApi31Converter and check/insert the refName(ref) at the start of
convertResolvedSchemaReference to short-circuit and return null (or a sentinel)
if already seen, and remove the name on exit; ensure
convertSchemaOrReference/convertSchema/convertAllOfSchema continue to call
convertResolvedSchemaReference so the guard prevents cycles; update any other
entry points (the overloaded range around lines 258-272) to use the same
visited-set.
- Around line 258-296: convertAllOfSchema leaks merged_properties on errors and
duplicates required entries from inline schema: add an errdefer to deinit
merged_properties (and ensure any allocated keys/schemas already inserted are
cleaned on error) immediately after initializing merged_properties so
mergeProperties, mergeRequired, convertSchemaOrReference, or any failure in the
inline-properties loop will free the hashmap; and when handling schema.required
(the inline block that appends into required_list), call the same deduping
helper (mergeRequired) or reuse its logic instead of raw append so required
items present both in allOf members and the parent schema are deduplicated
(refer to merged_properties, required_list, mergeProperties, mergeRequired,
convertAllOfSchema, convertSchemaOrReference).
In `@src/tests/resource_wrapper_tests.zig`:
- Line 69: Tests currently use std.testing.allocator instead of the project's
test helper; replace the usage of the bare allocator (const allocator) in
resource_wrapper_tests.zig with test_utils.createTestAllocator() and import the
test_utils module; call test_utils.createTestAllocator() at test setup and use
its returned allocator for allocations so leak detection follows the project's
convention (reference: test_utils.createTestAllocator, const allocator in the
test function).
---
Outside diff comments:
In `@src/models/v3.1/schema.zig`:
- Around line 134-137: The exclusiveMaximum and exclusiveMinimum fields are
incorrectly typed as ?bool and parsed with optionalBool(); change their types to
?f64 (numeric optional) in the schema definition (alongside maximum and minimum)
and switch parsing/serialization to use the numeric optional helper (e.g.,
optionalNumber()) instead of optionalBool(); update any code paths that
reference exclusiveMaximum/exclusiveMinimum (and the optionalBool() usage in
this context) so numeric values like 0 are preserved for OpenAPI 3.1 / JSON
Schema 2020-12 compatibility.
---
Nitpick comments:
In `@docs/openai-generation-issues.md`:
- Around line 394-397: The three consecutive bullets all start with "Add", which
is repetitive; rephrase them for variety by turning one or two into alternative
verbs or sentence structures—e.g., "Support extra_body flattening", "Introduce
reasoning_details extension support", "Implement an initial streaming/SSE
runtime", and "Begin multipart support or mark unsupported endpoints as
raw"—update the lines containing the phrases "extra_body flattening",
"reasoning_details extension support", "first streaming/SSE runtime", and
"multipart support or mark unsupported endpoints raw" accordingly.
- Around line 58-60: The three bullets all begin with the word "Generated",
which is monotonous; reword them to vary sentence openings while preserving
meaning—e.g., start the first with "The generated runtime exposes ..." or
"Runtime exposes ..." for RawResponse / requestRaw / getRaw / postJsonRaw,
change the second to "It also includes a bounded dynamic SSE parser" and the
third to "Supported line endings: LF and CRLF" (or similar) so each bullet uses
a different lead phrase but still references the same symbols (RawResponse,
requestRaw, getRaw, postJsonRaw, SSE parser, LF, CRLF).
In `@src/cli.zig`:
- Line 15: The default value for the CLI option resource_wrappers
(ResourceWrapperMode) is set to .paths which changes behavior for existing
users; change the default to .none to preserve backward compatibility, update
any code that references resource_wrappers: ResourceWrapperMode = .paths to use
.none instead, and update the usage text in printUsage() to indicate "(default:
none)"; ensure the CLI flag parsing still accepts --resource-wrappers paths so
users can opt in to .paths behavior when desired.
In `@src/generators/converters/openapi32_converter.zig`:
- Around line 339-353: The fallback that uses content.iterator().next() is
non-deterministic; update convertRequestBody and convertResponse (and analogous
logic in openapi31_converter.zig and openapi_converter.zig) to deterministically
pick a media type: after checking "application/json", build a list of available
content keys, then search them in a preferred-order list (e.g.,
"application/xml", "text/xml", "application/x-yaml", "text/yaml") and use the
first match; if none match, sort the keys (lexicographically) and pick the first
entry so output is stable across runs; replace the iterator().next() usage with
this deterministic selection and then call convertSchemaOrReference on the
chosen media type's schema.
In `@src/tests/resource_wrapper_tests.zig`:
- Around line 16-43: Remove the unused method parameter from the op function
signature and all its call sites (e.g., in buildFixture) so op only accepts
(allocator, operation_id, has_body, has_path_param, has_response); update
references to the symbol op and any callers to stop passing the method argument
(or alternatively, if you prefer to keep method, wire it into op's returned
operation fields instead). Ensure you update the op declaration and every
invocation site in tests/helpers (such as buildFixture) to match the new
parameter list to fix the unused-variable discard (_ = method).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8a00b704-97ba-4ffc-a0da-62b322cefc5a
⛔ Files ignored due to path filters (6)
generated/compile_generated.zigis excluded by!**/generated/**generated/generated_v2.zigis excluded by!**/generated/**generated/generated_v3.zigis excluded by!**/generated/**generated/generated_v31.zigis excluded by!**/generated/**generated/generated_v32.zigis excluded by!**/generated/**generated/main.zigis excluded by!**/generated/**
📒 Files selected for processing (10)
docs/openai-generation-issues.mdsrc/cli.zigsrc/generators/converters/openapi31_converter.zigsrc/generators/converters/openapi32_converter.zigsrc/generators/converters/openapi_converter.zigsrc/generators/unified/api_generator.zigsrc/generators/unified/model_generator.zigsrc/models/v3.1/schema.zigsrc/tests.zigsrc/tests/resource_wrapper_tests.zig
✅ Files skipped from review due to trivial changes (2)
- src/tests.zig
- src/generators/unified/model_generator.zig
Comment on lines
+183
to
+187
| fn convertResolvedSchemaReference(self: *OpenApi31Converter, ref: []const u8) anyerror!?Schema { | ||
| const source_schemas = self.source_schemas orelse return null; | ||
| const schema_or_ref = source_schemas.get(refName(ref)) orelse return null; | ||
| return try self.convertSchemaOrReference(schema_or_ref); | ||
| } |
Contributor
There was a problem hiding this comment.
Cyclic allOf references can cause unbounded recursion / stack overflow.
convertResolvedSchemaReference resolves a $ref and re-enters convertSchemaOrReference → convertSchema → convertAllOfSchema. If two component schemas reference each other via allOf (e.g., A.allOf: [{ $ref: B }], B.allOf: [{ $ref: A }] — present in some real-world specs), this recurses without termination. Real OpenAPI specs (the PR target) sometimes contain such cycles; recommend a visited-set guard keyed on the resolved component name.
🐛 Sketch of a fix
pub const OpenApi31Converter = struct {
allocator: std.mem.Allocator,
source_schemas: ?*const std.StringHashMap(SchemaOrReference31) = null,
+ visiting: std.StringHashMap(void) = undefined, // init in init()
...
fn convertResolvedSchemaReference(self: *OpenApi31Converter, ref: []const u8) anyerror!?Schema {
const source_schemas = self.source_schemas orelse return null;
- const schema_or_ref = source_schemas.get(refName(ref)) orelse return null;
- return try self.convertSchemaOrReference(schema_or_ref);
+ const name = refName(ref);
+ if (self.visiting.contains(name)) return Schema{ .type = .reference, .ref = ref };
+ const schema_or_ref = source_schemas.get(name) orelse return null;
+ try self.visiting.put(name, {});
+ defer _ = self.visiting.remove(name);
+ return try self.convertSchemaOrReference(schema_or_ref);
}Also applies to: 258-272
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/generators/converters/openapi31_converter.zig` around lines 183 - 187,
convertResolvedSchemaReference currently follows $ref into
convertSchemaOrReference which can re-enter via convertSchema ->
convertAllOfSchema and lead to infinite recursion for cyclic allOf references;
add a visited-set (e.g., a HashMap or Set) of resolved component names stored on
OpenApi31Converter and check/insert the refName(ref) at the start of
convertResolvedSchemaReference to short-circuit and return null (or a sentinel)
if already seen, and remove the name on exit; ensure
convertSchemaOrReference/convertSchema/convertAllOfSchema continue to call
convertResolvedSchemaReference so the guard prevents cycles; update any other
entry points (the overloaded range around lines 258-272) to use the same
visited-set.
Comment on lines
+258
to
+296
| fn convertAllOfSchema(self: *OpenApi31Converter, schema: Schema31) anyerror!Schema { | ||
| var merged_properties = std.StringHashMap(Schema).init(self.allocator); | ||
| var required_list = std.ArrayList([]const u8).empty; | ||
|
|
||
| if (schema.allOf) |all_of| { | ||
| for (all_of) |item| { | ||
| var converted = switch (item) { | ||
| .reference => |ref| (try self.convertResolvedSchemaReference(ref.ref)) orelse Schema{ .type = .reference, .ref = ref.ref }, | ||
| .schema => |child| try self.convertSchema(child.*), | ||
| }; | ||
| try self.mergeProperties(&merged_properties, converted); | ||
| try self.mergeRequired(&required_list, converted.required); | ||
| converted.deinit(self.allocator); | ||
| } | ||
| } | ||
|
|
||
| if (schema.properties) |props| { | ||
| var prop_iterator = props.iterator(); | ||
| while (prop_iterator.next()) |entry| { | ||
| const prop_schema = try self.convertSchemaOrReference(entry.value_ptr.*); | ||
| if (merged_properties.getEntry(entry.key_ptr.*)) |existing| { | ||
| existing.value_ptr.deinit(self.allocator); | ||
| existing.value_ptr.* = prop_schema; | ||
| } else { | ||
| const key = try self.allocator.dupe(u8, entry.key_ptr.*); | ||
| try merged_properties.put(key, prop_schema); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (schema.required) |required| { | ||
| for (required) |item| try required_list.append(self.allocator, item); | ||
| } | ||
|
|
||
| const required = if (required_list.items.len > 0) try required_list.toOwnedSlice(self.allocator) else null; | ||
| const has_properties = merged_properties.count() > 0; | ||
| if (!has_properties) { | ||
| merged_properties.deinit(); | ||
| } |
Contributor
There was a problem hiding this comment.
Minor cleanups in convertAllOfSchema.
Two small issues:
merged_propertiesis initialized but lackserrdefercleanup. IfmergeProperties,mergeRequired, or the inline-properties loop fails, the hashmap (and any cloned schemas already inserted) leaks.- The inline
schema.requiredis appended without dedup (line 288-290), while allOf-derived requireds usemergeRequiredwhich dedups. A required field listed both in an allOf member and on the parent schema will appear twice in the merged output.
🛠️ Suggested fix
fn convertAllOfSchema(self: *OpenApi31Converter, schema: Schema31) anyerror!Schema {
var merged_properties = std.StringHashMap(Schema).init(self.allocator);
+ errdefer {
+ var it = merged_properties.iterator();
+ while (it.next()) |entry| {
+ self.allocator.free(entry.key_ptr.*);
+ entry.value_ptr.deinit(self.allocator);
+ }
+ merged_properties.deinit();
+ }
var required_list = std.ArrayList([]const u8).empty;
+ errdefer required_list.deinit(self.allocator);
...
- if (schema.required) |required| {
- for (required) |item| try required_list.append(self.allocator, item);
- }
+ // Use mergeRequired-style dedup for consistency with allOf members.
+ if (schema.required) |required| {
+ for (required) |item| {
+ var exists = false;
+ for (required_list.items) |existing| {
+ if (std.mem.eql(u8, existing, item)) { exists = true; break; }
+ }
+ if (!exists) try required_list.append(self.allocator, item);
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fn convertAllOfSchema(self: *OpenApi31Converter, schema: Schema31) anyerror!Schema { | |
| var merged_properties = std.StringHashMap(Schema).init(self.allocator); | |
| var required_list = std.ArrayList([]const u8).empty; | |
| if (schema.allOf) |all_of| { | |
| for (all_of) |item| { | |
| var converted = switch (item) { | |
| .reference => |ref| (try self.convertResolvedSchemaReference(ref.ref)) orelse Schema{ .type = .reference, .ref = ref.ref }, | |
| .schema => |child| try self.convertSchema(child.*), | |
| }; | |
| try self.mergeProperties(&merged_properties, converted); | |
| try self.mergeRequired(&required_list, converted.required); | |
| converted.deinit(self.allocator); | |
| } | |
| } | |
| if (schema.properties) |props| { | |
| var prop_iterator = props.iterator(); | |
| while (prop_iterator.next()) |entry| { | |
| const prop_schema = try self.convertSchemaOrReference(entry.value_ptr.*); | |
| if (merged_properties.getEntry(entry.key_ptr.*)) |existing| { | |
| existing.value_ptr.deinit(self.allocator); | |
| existing.value_ptr.* = prop_schema; | |
| } else { | |
| const key = try self.allocator.dupe(u8, entry.key_ptr.*); | |
| try merged_properties.put(key, prop_schema); | |
| } | |
| } | |
| } | |
| if (schema.required) |required| { | |
| for (required) |item| try required_list.append(self.allocator, item); | |
| } | |
| const required = if (required_list.items.len > 0) try required_list.toOwnedSlice(self.allocator) else null; | |
| const has_properties = merged_properties.count() > 0; | |
| if (!has_properties) { | |
| merged_properties.deinit(); | |
| } | |
| fn convertAllOfSchema(self: *OpenApi31Converter, schema: Schema31) anyerror!Schema { | |
| var merged_properties = std.StringHashMap(Schema).init(self.allocator); | |
| errdefer { | |
| var it = merged_properties.iterator(); | |
| while (it.next()) |entry| { | |
| self.allocator.free(entry.key_ptr.*); | |
| entry.value_ptr.deinit(self.allocator); | |
| } | |
| merged_properties.deinit(); | |
| } | |
| var required_list = std.ArrayList([]const u8).empty; | |
| errdefer required_list.deinit(self.allocator); | |
| if (schema.allOf) |all_of| { | |
| for (all_of) |item| { | |
| var converted = switch (item) { | |
| .reference => |ref| (try self.convertResolvedSchemaReference(ref.ref)) orelse Schema{ .type = .reference, .ref = ref.ref }, | |
| .schema => |child| try self.convertSchema(child.*), | |
| }; | |
| try self.mergeProperties(&merged_properties, converted); | |
| try self.mergeRequired(&required_list, converted.required); | |
| converted.deinit(self.allocator); | |
| } | |
| } | |
| if (schema.properties) |props| { | |
| var prop_iterator = props.iterator(); | |
| while (prop_iterator.next()) |entry| { | |
| const prop_schema = try self.convertSchemaOrReference(entry.value_ptr.*); | |
| if (merged_properties.getEntry(entry.key_ptr.*)) |existing| { | |
| existing.value_ptr.deinit(self.allocator); | |
| existing.value_ptr.* = prop_schema; | |
| } else { | |
| const key = try self.allocator.dupe(u8, entry.key_ptr.*); | |
| try merged_properties.put(key, prop_schema); | |
| } | |
| } | |
| } | |
| // Use mergeRequired-style dedup for consistency with allOf members. | |
| if (schema.required) |required| { | |
| for (required) |item| { | |
| var exists = false; | |
| for (required_list.items) |existing| { | |
| if (std.mem.eql(u8, existing, item)) { exists = true; break; } | |
| } | |
| if (!exists) try required_list.append(self.allocator, item); | |
| } | |
| } | |
| const required = if (required_list.items.len > 0) try required_list.toOwnedSlice(self.allocator) else null; | |
| const has_properties = merged_properties.count() > 0; | |
| if (!has_properties) { | |
| merged_properties.deinit(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/generators/converters/openapi31_converter.zig` around lines 258 - 296,
convertAllOfSchema leaks merged_properties on errors and duplicates required
entries from inline schema: add an errdefer to deinit merged_properties (and
ensure any allocated keys/schemas already inserted are cleaned on error)
immediately after initializing merged_properties so mergeProperties,
mergeRequired, convertSchemaOrReference, or any failure in the inline-properties
loop will free the hashmap; and when handling schema.required (the inline block
that appends into required_list), call the same deduping helper (mergeRequired)
or reuse its logic instead of raw append so required items present both in allOf
members and the parent schema are deduplicated (refer to merged_properties,
required_list, mergeProperties, mergeRequired, convertAllOfSchema,
convertSchemaOrReference).
| } | ||
|
|
||
| test "resource wrappers derive from paths" { | ||
| const allocator = std.testing.allocator; |
Contributor
There was a problem hiding this comment.
Use test_utils.createTestAllocator() per the project's test pattern.
Per the project's test conventions, test functions should obtain the allocator via test_utils.createTestAllocator() rather than std.testing.allocator. Both detect leaks, but the helper keeps the test surface uniform with the rest of src/tests/**.
As per coding guidelines: "Use test_utils.createTestAllocator() in all test functions for leak detection".
🛠️ Suggested change
+const test_utils = `@import`("test_utils.zig");
...
test "resource wrappers derive from paths" {
- const allocator = std.testing.allocator;
+ var gpa = test_utils.createTestAllocator();
+ const allocator = gpa.allocator();
var document = try buildFixture(allocator);
defer document.deinit(allocator);(Adjust the import path to wherever test_utils.zig lives in this repo.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tests/resource_wrapper_tests.zig` at line 69, Tests currently use
std.testing.allocator instead of the project's test helper; replace the usage of
the bare allocator (const allocator) in resource_wrapper_tests.zig with
test_utils.createTestAllocator() and import the test_utils module; call
test_utils.createTestAllocator() at test setup and use its returned allocator
for allocations so leak detection follows the project's convention (reference:
test_utils.createTestAllocator, const allocator in the test function).
5dc1078 to
b1e6b12
Compare
h0rv
force-pushed
the
fix-real-spec-generation
branch
3 times, most recently
from
436feb2 to
97ce8f6
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
src/generators/unified/api_generator.zig (1)
1247-1253:⚠️ Potential issue | 🔴 CriticalFallback operation identifier still ignores
method(and is unescaped).When
operationIdis missing, the emitted name is@"operation" ++ path[1..]. Two operations on the same path (e.g.,GET /users+POST /users) collide into the samepub fn @"operationusers", producing duplicate-symbol compile errors.path[1..]is also appended raw inside@"…"with no escape handling for"or\. A previous review thread on this exact concern is marked "addressed", but the current code still exhibits the bug — please route throughappendIdentifierand includemethod.🐛 Suggested fix
if (operation.operationId) |op_id| { try self.appendIdentifier(op_id); } else { - try self.buffer.appendSlice(self.allocator, "@\"operation"); - try self.buffer.appendSlice(self.allocator, path[1..]); - try self.buffer.appendSlice(self.allocator, "\""); + var name_buf: std.ArrayList(u8) = .empty; + defer name_buf.deinit(self.allocator); + try name_buf.appendSlice(self.allocator, method); + try name_buf.append(self.allocator, '_'); + try name_buf.appendSlice(self.allocator, path[1..]); + try self.appendIdentifier(name_buf.items); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/generators/unified/api_generator.zig` around lines 1247 - 1253, The fallback builds a raw @"operation" + path string which ignores the HTTP method and skips escaping, causing symbol collisions and unescaped quotes/backslashes; instead of appending raw slices (buffer.appendSlice) in the else branch, construct a fallback name that includes the operation method and path and pass it through appendIdentifier (e.g., combine method and path[1..] into a single identifier string) so escaping and unique naming are handled by appendIdentifier rather than emitting @"operation"+path directly.src/tests/resource_wrapper_tests.zig (1)
68-77:⚠️ Potential issue | 🟡 MinorUse
test_utils.createTestAllocator()instead ofstd.testing.allocator.Per the project's test convention, all tests under
src/tests/**should obtain the allocator viatest_utils.createTestAllocator()for uniform leak detection. As per coding guidelines: "Usetest_utils.createTestAllocator()in all tests for leak detection".🛠️ Suggested change
+const test_utils = `@import`("test_utils.zig"); @@ test "resource wrappers derive from paths" { - const allocator = std.testing.allocator; + var gpa = test_utils.createTestAllocator(); + const allocator = gpa.allocator(); var document = try buildFixture(allocator); defer document.deinit(allocator);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tests/resource_wrapper_tests.zig` around lines 68 - 77, Replace usage of std.testing.allocator with the project's test allocator by calling test_utils.createTestAllocator() at the start of the test "resource wrappers derive from paths"; use that allocator for buildFixture(allocator) and UnifiedApiGenerator.init(...), and ensure you call the allocator's destroy/free per convention (or the test_utils-provided cleanup) before deferring document.deinit(allocator) and generator.deinit() so the test uses the uniform allocator used for leak detection.src/generators/converters/openapi31_converter.zig (1)
272-310:⚠️ Potential issue | 🟡 MinorCleanup gaps in
convertAllOfSchema(errdefer + inline-required dedup).Two pre-existing concerns from a prior review still apply:
merged_properties(line 273) andrequired_list(line 274) lackerrdefercleanup; if any ofmergeProperties/mergeRequired/convertSchemaOrReference(line 291) /required_list.append(line 303) fails after theallOfloop has populated entries, the partially built map and any cloned schemas inside it leak.- The inline
schema.requiredappend (lines 302–304) bypassesmergeRequired's dedup, so a field listed both in anallOfmember and on the parent schema appears twice in the mergedrequiredslice.Cyclic
allOf$refchains (A.allOf:[$ref:B]/B.allOf:[$ref:A]) can also still recurse without termination viaconvertResolvedSchemaReference→convertSchema→convertAllOfSchema; a visited-set keyed on the resolved component name would terminate this safely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/generators/converters/openapi31_converter.zig` around lines 272 - 310, convertAllOfSchema currently leaks memory on errors and duplicates required entries; ensure merged_properties and required_list are cleaned up on any error by adding errdefer blocks that deinit merged_properties and call required_list.deinit(), and when cleaning merged_properties also deinit any stored Schema values (use the same deinit path used elsewhere) so partially converted schemas don't leak; replace the inline appends of schema.required with a call to mergeRequired(&required_list, schema.required) so dedup logic is reused (refer to mergeRequired), and add a visited-set (e.g. a std.StringHashSet or similar) passed into convertResolvedSchemaReference/convertSchema/convertAllOfSchema to detect and short-circuit cyclic allOf $ref chains by returning a reference or error when a component name is already visited.docs/openai-generation-issues.md (1)
83-230:⚠️ Potential issue | 🟡 Minor"Remaining issues"
#2–#5 contradict the "Update: streaming/OpenRouter usability pass" section above and the actual implementation.The doc still lists as remaining:
#2Response parsing fails on unknown provider fields — but line 57 says parsing already uses.ignore_unknown_fields = true, andapi_generator.zigline 1467 confirms it.#3Noreasoning_details— but line 74 says assistant message structs include it.#4Noextra_bodyflattening — but line 73 saysCreateChatCompletionRequest/CreateResponseinclude flattenedextra_body, with code inmodel_generator.zig.#5Streaming not implemented — but lines 59–72 enumerate the bounded SSE parser,streamChatCompletion/streamResponse, and successful OpenRouter live test, andapi_generator.zigemitsparseSseReaderetc.Either remove these four entries, or rewrite them to point only at the specific scenarios that still fail (e.g., typed event parsing for
#5).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openai-generation-issues.md` around lines 83 - 230, The docs section "Remaining issues" incorrectly lists items `#2`–#5 as unresolved; update docs to reflect the implemented behavior: remove or rewrite entries `#2`–#5 to either delete them or narrow them to the remaining edge-cases (e.g., typed event parsing only). Reference the actual implementations when editing: confirm JSON parsing uses std.json.parseFromSlice(..., .{ .ignore_unknown_fields = true }) emitted by api_generator (parseFromSlice invocation), confirm reasoning_details and extra_body are present in generated structs (CreateChatCompletionRequest / CreateResponse in model_generator), and confirm streaming support exists (parseSseReader, streamChatCompletion, streamResponse). Ensure the doc text points to these symbols/behaviors and only keeps notes about specific failing scenarios instead of blanket "not implemented".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/generators/converters/openapi31_converter.zig`:
- Around line 348-375: convertUnionSchema can leak the allocated refs from
convertUnionRefs if allocator.dupe(u8, discriminator.propertyName) fails; modify
convertUnionSchema to set up an errdefer that frees the refs slice and its inner
strings (the allocations returned by convertUnionRefs) before calling
self.allocator.dupe, and cancel that errdefer after duping succeeds so
successful returns (the Schema constructions for one_of_refs and any_of_refs)
don't double-free; reference the convertUnionSchema function, the
convertUnionRefs call that produces refs, the discriminator.propertyName dup via
self.allocator.dupe, and ensure the errdefer cleanup releases both the outer
slice and each inner u8 slice.
In `@src/generators/unified/api_generator.zig`:
- Around line 224-228: The emitted Zig source writes self.args.base_url raw
between string delimiters using buffer.appendSlice, which will break if base_url
contains quotes, backslashes, or control chars; update the generator to escape
the base_url before embedding it by reusing the escaping logic from
appendIdentifier (or factor that logic into a new helper like
appendStringLiteralBody) and call that helper instead of appendSlice for
self.args.base_url; ensure it handles '\\', '"' and common escapes '\n', '\r',
'\t' so the produced base_url: []const u8 = "<escaped value>" is valid Zig
source.
In `@src/tests/model_typing_tests.zig`:
- Around line 11-189: Replace std.testing.allocator with
test_utils.createTestAllocator() in the four tests ("model generator treats
properties without type as struct", "OpenAPI 3.1 allOf object refs merge into
one schema", "OpenAPI 3.1 discriminator oneOf emits tagged union with raw
fallback", "extensible request structs emit extra_body merge hook") so the tests
use the test allocator for leak detection; update each test to call
createTestAllocator(), defer its destroy if required, and pass that allocator
into any allocations (e.g., StringHashMap init and generator init). In the
fourth test, remove the manual allocator.free of
schemas.getPtr("CreateChatCompletionRequest").?.required.? and instead use a
compile-time literal slice for required (e.g., &[_][]const u8{"model"}) so no
manual free is needed and you avoid potential double-free if Schema.deinit is
later invoked; ensure references to schemas, properties, generator, and
UnifiedModelGenerator.init remain unchanged except for allocator usage.
---
Duplicate comments:
In `@docs/openai-generation-issues.md`:
- Around line 83-230: The docs section "Remaining issues" incorrectly lists
items `#2`–#5 as unresolved; update docs to reflect the implemented behavior:
remove or rewrite entries `#2`–#5 to either delete them or narrow them to the
remaining edge-cases (e.g., typed event parsing only). Reference the actual
implementations when editing: confirm JSON parsing uses
std.json.parseFromSlice(..., .{ .ignore_unknown_fields = true }) emitted by
api_generator (parseFromSlice invocation), confirm reasoning_details and
extra_body are present in generated structs (CreateChatCompletionRequest /
CreateResponse in model_generator), and confirm streaming support exists
(parseSseReader, streamChatCompletion, streamResponse). Ensure the doc text
points to these symbols/behaviors and only keeps notes about specific failing
scenarios instead of blanket "not implemented".
In `@src/generators/converters/openapi31_converter.zig`:
- Around line 272-310: convertAllOfSchema currently leaks memory on errors and
duplicates required entries; ensure merged_properties and required_list are
cleaned up on any error by adding errdefer blocks that deinit merged_properties
and call required_list.deinit(), and when cleaning merged_properties also deinit
any stored Schema values (use the same deinit path used elsewhere) so partially
converted schemas don't leak; replace the inline appends of schema.required with
a call to mergeRequired(&required_list, schema.required) so dedup logic is
reused (refer to mergeRequired), and add a visited-set (e.g. a std.StringHashSet
or similar) passed into
convertResolvedSchemaReference/convertSchema/convertAllOfSchema to detect and
short-circuit cyclic allOf $ref chains by returning a reference or error when a
component name is already visited.
In `@src/generators/unified/api_generator.zig`:
- Around line 1247-1253: The fallback builds a raw @"operation" + path string
which ignores the HTTP method and skips escaping, causing symbol collisions and
unescaped quotes/backslashes; instead of appending raw slices
(buffer.appendSlice) in the else branch, construct a fallback name that includes
the operation method and path and pass it through appendIdentifier (e.g.,
combine method and path[1..] into a single identifier string) so escaping and
unique naming are handled by appendIdentifier rather than emitting
@"operation"+path directly.
In `@src/tests/resource_wrapper_tests.zig`:
- Around line 68-77: Replace usage of std.testing.allocator with the project's
test allocator by calling test_utils.createTestAllocator() at the start of the
test "resource wrappers derive from paths"; use that allocator for
buildFixture(allocator) and UnifiedApiGenerator.init(...), and ensure you call
the allocator's destroy/free per convention (or the test_utils-provided cleanup)
before deferring document.deinit(allocator) and generator.deinit() so the test
uses the uniform allocator used for leak detection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d8caf37d-016e-431c-9378-84c089ef8a76
⛔ Files ignored due to path filters (6)
generated/compile_generated.zigis excluded by!**/generated/**generated/generated_v2.zigis excluded by!**/generated/**generated/generated_v3.zigis excluded by!**/generated/**generated/generated_v31.zigis excluded by!**/generated/**generated/generated_v32.zigis excluded by!**/generated/**generated/main.zigis excluded by!**/generated/**
📒 Files selected for processing (13)
docs/json-value-typing-policy.mddocs/openai-generation-issues.mdsrc/cli.zigsrc/generators/converters/openapi31_converter.zigsrc/generators/converters/openapi32_converter.zigsrc/generators/converters/openapi_converter.zigsrc/generators/unified/api_generator.zigsrc/generators/unified/model_generator.zigsrc/models/common/document.zigsrc/models/v3.1/schema.zigsrc/tests.zigsrc/tests/model_typing_tests.zigsrc/tests/resource_wrapper_tests.zig
✅ Files skipped from review due to trivial changes (3)
- src/tests.zig
- docs/json-value-typing-policy.md
- src/generators/converters/openapi32_converter.zig
🚧 Files skipped from review as they are similar to previous changes (3)
- src/generators/converters/openapi_converter.zig
- src/models/v3.1/schema.zig
- src/generators/unified/model_generator.zig
Comment on lines
+348
to
+375
| fn convertUnionSchema(self: *OpenApi31Converter, schema: Schema31) anyerror!Schema { | ||
| const discriminator = schema.discriminator orelse return Schema{ .type = .object }; | ||
|
|
||
| if (schema.oneOf) |one_of| { | ||
| if (try self.convertUnionRefs(one_of)) |refs| { | ||
| return Schema{ | ||
| .type = .object, | ||
| .one_of_refs = refs, | ||
| .discriminator_property = try self.allocator.dupe(u8, discriminator.propertyName), | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| if (schema.anyOf) |any_of| { | ||
| if (try self.convertUnionRefs(any_of)) |refs| { | ||
| return Schema{ | ||
| .type = .object, | ||
| .any_of_refs = refs, | ||
| .discriminator_property = try self.allocator.dupe(u8, discriminator.propertyName), | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| return Schema{ | ||
| .type = .object, | ||
| .description = "OpenAPI oneOf with discriminator could not be generated safely; generator currently uses std.json.Value.", | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
convertUnionSchema leaks refs if duping discriminator.propertyName fails.
convertUnionRefs returns a slice with each entry allocator-duped plus the outer slice allocated. If try self.allocator.dupe(u8, discriminator.propertyName) then fails (OOM), refs is dropped on the floor — neither inner strings nor outer slice are freed. Add an errdefer covering both branches before duping the discriminator property.
🛡️ Suggested fix
if (schema.oneOf) |one_of| {
if (try self.convertUnionRefs(one_of)) |refs| {
+ errdefer {
+ for (refs) |r| self.allocator.free(r);
+ self.allocator.free(refs);
+ }
return Schema{
.type = .object,
.one_of_refs = refs,
.discriminator_property = try self.allocator.dupe(u8, discriminator.propertyName),
};
}
}
if (schema.anyOf) |any_of| {
if (try self.convertUnionRefs(any_of)) |refs| {
+ errdefer {
+ for (refs) |r| self.allocator.free(r);
+ self.allocator.free(refs);
+ }
return Schema{
.type = .object,
.any_of_refs = refs,
.discriminator_property = try self.allocator.dupe(u8, discriminator.propertyName),
};
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/generators/converters/openapi31_converter.zig` around lines 348 - 375,
convertUnionSchema can leak the allocated refs from convertUnionRefs if
allocator.dupe(u8, discriminator.propertyName) fails; modify convertUnionSchema
to set up an errdefer that frees the refs slice and its inner strings (the
allocations returned by convertUnionRefs) before calling self.allocator.dupe,
and cancel that errdefer after duping succeeds so successful returns (the Schema
constructions for one_of_refs and any_of_refs) don't double-free; reference the
convertUnionSchema function, the convertUnionRefs call that produces refs, the
discriminator.propertyName dup via self.allocator.dupe, and ensure the errdefer
cleanup releases both the outer slice and each inner u8 slice.
Comment on lines
+11
to
+189
| test "model generator treats properties without type as struct" { | ||
| const allocator = std.testing.allocator; | ||
| var properties = std.StringHashMap(common.Schema).init(allocator); | ||
| defer { | ||
| var iterator = properties.iterator(); | ||
| while (iterator.next()) |entry| allocator.free(entry.key_ptr.*); | ||
| properties.deinit(); | ||
| } | ||
| try properties.put(try allocator.dupe(u8, "foo"), stringSchema()); | ||
|
|
||
| var schemas = std.StringHashMap(common.Schema).init(allocator); | ||
| defer { | ||
| var iterator = schemas.iterator(); | ||
| while (iterator.next()) |entry| allocator.free(entry.key_ptr.*); | ||
| schemas.deinit(); | ||
| } | ||
| try schemas.put(try allocator.dupe(u8, "Foo"), .{ .properties = properties }); | ||
|
|
||
| var paths = std.StringHashMap(common.PathItem).init(allocator); | ||
| defer paths.deinit(); | ||
| const document: common.UnifiedDocument = .{ | ||
| .version = "3.1.0", | ||
| .info = .{ .title = "fixture", .version = "1.0.0" }, | ||
| .paths = paths, | ||
| .schemas = schemas, | ||
| }; | ||
|
|
||
| var generator = UnifiedModelGenerator.init(allocator); | ||
| defer generator.deinit(); | ||
| const code = try generator.generate(document); | ||
| defer allocator.free(code); | ||
|
|
||
| try std.testing.expect(std.mem.indexOf(u8, code, "pub const Foo = struct") != null); | ||
| try std.testing.expect(std.mem.indexOf(u8, code, "foo: ?[]const u8 = null") != null); | ||
| } | ||
|
|
||
| test "OpenAPI 3.1 allOf object refs merge into one schema" { | ||
| const allocator = std.testing.allocator; | ||
| const source = | ||
| \\{ | ||
| \\ "openapi": "3.1.0", | ||
| \\ "info": { "title": "fixture", "version": "1.0.0" }, | ||
| \\ "paths": {}, | ||
| \\ "components": { | ||
| \\ "schemas": { | ||
| \\ "Base": { | ||
| \\ "type": "object", | ||
| \\ "required": ["id"], | ||
| \\ "properties": { "id": { "type": "string" } } | ||
| \\ }, | ||
| \\ "Thing": { | ||
| \\ "allOf": [ | ||
| \\ { "$ref": "#/components/schemas/Base" }, | ||
| \\ { | ||
| \\ "type": "object", | ||
| \\ "required": ["name"], | ||
| \\ "properties": { "name": { "type": "string" } } | ||
| \\ } | ||
| \\ ] | ||
| \\ } | ||
| \\ } | ||
| \\ } | ||
| \\} | ||
| ; | ||
|
|
||
| var parsed = try models.OpenApi31Document.parseFromJson(allocator, source); | ||
| defer parsed.deinit(allocator); | ||
|
|
||
| var converter = OpenApi31Converter.init(allocator); | ||
| var unified = try converter.convert(parsed); | ||
| defer unified.deinit(allocator); | ||
|
|
||
| const schemas = unified.schemas.?; | ||
| const thing = schemas.get("Thing").?; | ||
| try std.testing.expect(thing.properties != null); | ||
| try std.testing.expect(thing.properties.?.contains("id")); | ||
| try std.testing.expect(thing.properties.?.contains("name")); | ||
| try std.testing.expect(thing.required != null); | ||
| try std.testing.expectEqual(@as(usize, 2), thing.required.?.len); | ||
| } | ||
|
|
||
| test "OpenAPI 3.1 discriminator oneOf emits tagged union with raw fallback" { | ||
| const allocator = std.testing.allocator; | ||
| const source = | ||
| \\{ | ||
| \\ "openapi": "3.1.0", | ||
| \\ "info": { "title": "fixture", "version": "1.0.0" }, | ||
| \\ "paths": {}, | ||
| \\ "components": { | ||
| \\ "schemas": { | ||
| \\ "Cat": { | ||
| \\ "type": "object", | ||
| \\ "required": ["type", "meows"], | ||
| \\ "properties": { | ||
| \\ "type": { "type": "string", "enum": ["cat"] }, | ||
| \\ "meows": { "type": "boolean" } | ||
| \\ } | ||
| \\ }, | ||
| \\ "Dog": { | ||
| \\ "type": "object", | ||
| \\ "required": ["type", "barks"], | ||
| \\ "properties": { | ||
| \\ "type": { "type": "string", "enum": ["dog"] }, | ||
| \\ "barks": { "type": "boolean" } | ||
| \\ } | ||
| \\ }, | ||
| \\ "Pet": { | ||
| \\ "oneOf": [ | ||
| \\ { "$ref": "#/components/schemas/Cat" }, | ||
| \\ { "$ref": "#/components/schemas/Dog" } | ||
| \\ ], | ||
| \\ "discriminator": { "propertyName": "type" } | ||
| \\ } | ||
| \\ } | ||
| \\ } | ||
| \\} | ||
| ; | ||
|
|
||
| var parsed = try models.OpenApi31Document.parseFromJson(allocator, source); | ||
| defer parsed.deinit(allocator); | ||
|
|
||
| var converter = OpenApi31Converter.init(allocator); | ||
| var unified = try converter.convert(parsed); | ||
| defer unified.deinit(allocator); | ||
|
|
||
| var generator = UnifiedModelGenerator.init(allocator); | ||
| defer generator.deinit(); | ||
| const code = try generator.generate(unified); | ||
| defer allocator.free(code); | ||
|
|
||
| try std.testing.expect(std.mem.indexOf(u8, code, "pub const Pet = union(enum)") != null); | ||
| try std.testing.expect(std.mem.indexOf(u8, code, "cat: Cat") != null); | ||
| try std.testing.expect(std.mem.indexOf(u8, code, "dog: Dog") != null); | ||
| try std.testing.expect(std.mem.indexOf(u8, code, "raw: std.json.Value") != null); | ||
| try std.testing.expect(std.mem.indexOf(u8, code, "pub fn jsonParseFromValue") != null); | ||
| try std.testing.expect(std.mem.indexOf(u8, code, "source.object.get(\"type\")") != null); | ||
| } | ||
|
|
||
| test "extensible request structs emit extra_body merge hook" { | ||
| const allocator = std.testing.allocator; | ||
| var properties = std.StringHashMap(common.Schema).init(allocator); | ||
| defer { | ||
| var iterator = properties.iterator(); | ||
| while (iterator.next()) |entry| allocator.free(entry.key_ptr.*); | ||
| properties.deinit(); | ||
| } | ||
| try properties.put(try allocator.dupe(u8, "model"), stringSchema()); | ||
|
|
||
| var schemas = std.StringHashMap(common.Schema).init(allocator); | ||
| defer { | ||
| var iterator = schemas.iterator(); | ||
| while (iterator.next()) |entry| allocator.free(entry.key_ptr.*); | ||
| schemas.deinit(); | ||
| } | ||
| try schemas.put(try allocator.dupe(u8, "CreateChatCompletionRequest"), .{ | ||
| .type = .object, | ||
| .properties = properties, | ||
| .required = try allocator.dupe([]const u8, &.{"model"}), | ||
| }); | ||
| defer allocator.free(schemas.getPtr("CreateChatCompletionRequest").?.required.?); | ||
|
|
||
| var paths = std.StringHashMap(common.PathItem).init(allocator); | ||
| defer paths.deinit(); | ||
| const document: common.UnifiedDocument = .{ | ||
| .version = "3.1.0", | ||
| .info = .{ .title = "fixture", .version = "1.0.0" }, | ||
| .paths = paths, | ||
| .schemas = schemas, | ||
| }; | ||
|
|
||
| var generator = UnifiedModelGenerator.init(allocator); | ||
| defer generator.deinit(); | ||
| const code = try generator.generate(document); | ||
| defer allocator.free(code); | ||
|
|
||
| try std.testing.expect(std.mem.indexOf(u8, code, "extra_body: ?std.json.Value = null") != null); | ||
| try std.testing.expect(std.mem.indexOf(u8, code, "var iterator = extra.object.iterator()") != null); | ||
| try std.testing.expect(std.mem.indexOf(u8, code, "try jw.objectField(entry.key_ptr.*)") != null); | ||
| } |
Contributor
There was a problem hiding this comment.
Use test_utils.createTestAllocator() in all four tests.
All four tests (model generator treats properties without type as struct, OpenAPI 3.1 allOf object refs merge into one schema, OpenAPI 3.1 discriminator oneOf emits tagged union with raw fallback, extensible request structs emit extra_body merge hook) use std.testing.allocator. As per coding guidelines: "Use test_utils.createTestAllocator() in all tests for leak detection".
Also, the pattern in the fourth test (line 168/170) of allocating schemas.getPtr(...).?.required.? and manually allocator.free-ing it side-steps Schema.deinit. It works because the document is never deinited as a whole, but it's brittle: any future Schema.deinit call on this entry would double-free. Consider using a string literal slice (&[_][]const u8{"model"}) directly so no manual free is needed.
🛠️ Suggested change (per test)
+const test_utils = `@import`("test_utils.zig");
@@
test "model generator treats properties without type as struct" {
- const allocator = std.testing.allocator;
+ var gpa = test_utils.createTestAllocator();
+ const allocator = gpa.allocator();…and similarly for the other three tests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tests/model_typing_tests.zig` around lines 11 - 189, Replace
std.testing.allocator with test_utils.createTestAllocator() in the four tests
("model generator treats properties without type as struct", "OpenAPI 3.1 allOf
object refs merge into one schema", "OpenAPI 3.1 discriminator oneOf emits
tagged union with raw fallback", "extensible request structs emit extra_body
merge hook") so the tests use the test allocator for leak detection; update each
test to call createTestAllocator(), defer its destroy if required, and pass that
allocator into any allocations (e.g., StringHashMap init and generator init). In
the fourth test, remove the manual allocator.free of
schemas.getPtr("CreateChatCompletionRequest").?.required.? and instead use a
compile-time literal slice for required (e.g., &[_][]const u8{"model"}) so no
manual free is needed and you avoid potential double-free if Schema.deinit is
later invoked; ensure references to schemas, properties, generator, and
UnifiedModelGenerator.init remain unchanged except for allocator usage.
8a6208d to
efeb5a8
Compare
| \\ (default: generated.zig) | ||
| \\ --base-url <url> Base URL for the API client. | ||
| \\ (default: server URL from OpenAPI Specification) | ||
| \\ --resource-wrappers <mode> Generate resource wrappers: none, tags, paths, hybrid. |
Owner
There was a problem hiding this comment.
@h0rv Can you elaborate on this feature? Why and how would you use it?
Contributor
Author
There was a problem hiding this comment.
Yes - this is mostly just to get the nicer namespaced API:
var completion = try openai.chat.completions.create(&client, .{
.model = "gpt-5.4-mini",
.messages = &messages,
});instead of something flatter like:
try openai.chatCompletionsCreate(...);I wanted it to feel closer to the official OpenAI clients, like Python:
client.chat.completions.create(...)So yeah, mostly ergonomics/style, not really a correctness thing.
For --resource-wrappers:
none = flat functions
tags = group by OpenAPI tags
paths = group by path segments
hybrid = try to use whichever gives the nicest shape
For OpenAI, the namespaced style feels a lot better because the API already maps pretty naturally to things like chat.completions, responses, files, etc.
Owner
|
@h0rv Thanks for taking the time to implement this. It's a bit of a mouthful and I would like to thoroughly review it before we can merge something like in |
h0rv
force-pushed
the
fix-real-spec-generation
branch
2 times, most recently
from
d51fb7e to
ee4d293
Compare
h0rv
force-pushed
the
fix-real-spec-generation
branch
from
ee4d293 to
934f45a
Compare
Owner
|
@h0rv I pulled your branch and I've been going through this thoroughly. This is really good stuff! Do you mind sharing the process? You mentioned that this is agent-authored, I'm really impressed. I rarely get anything useful out of agentic engineering (obviously a skill issue) and I would love to see the process of how this was build, like the original prompt + session output? |
christianhelle
approved these changes
Apr 28, 2026
Contributor
Author
|
@christianhelle Thanks! Yeah, happy to share. The short version is: I wanted a real OpenAI Zig client that worked on Zig 0.16.0, and I did not want to hand-roll or maintain a generated-looking client manually. Your generator was already the best starting point I found, so I used the OpenAI OpenAPI spec as the real-world test case and iterated from there. The process was roughly:
The biggest thing I pushed for was stricter generated types: enums where possible, structured request/response types, fewer generic JSON blobs, and better support for real OpenAPI shapes used in the OpenAI spec. The other important piece was streaming/SSE support, since that is a core use case for AI clients and especially important for UX. The generated result is here if useful: https://github.com/h0rv/openai.zig The README there is probably the best summary of the shape I was aiming for. The process was basically “use a hard real-world spec, generate the client, try to use it seriously, then fix the generator until the generated client felt reasonable.” And I am using the pi coding agent btw with GPT-5.5 High - my config is here - its very simple: https://github.com/h0rv/dots/tree/main/pi. Session for openai.zig: https://pi.dev/session/#1e71b7d886f60349d145aec95a881593 |
Contributor
Author
|
And sorry the PR got so big 😄 - it got it to the point where the openai client is solid now. |
Owner
Normally a maintainer would get annoyed but the quality of this was too great to ignore. This project is also relatively new and isn't very well known so any contribution is a great help |
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.
Summary
Clientwith auth, base URL, and default headersapplication/jsonresponse schemas over streaming schemasallOfproperties for main request/response typesoneOf/anyOfmetadata and generate safe unionsstd.json.Value[]const std.json.Value?T, remove accidental??T, and collapse string-like unions to open stringsApiResult(T), parse-error raw preservation, and endpoint-specific raw/result helpersextra_bodyflattening andreasoning_detailssupportstd.json.Valuetyping policy,extra_body, and borrowed default headersStack
mainOpenAI type stats
std.json.Value total: 738pub const X = std.json.Valuealiases: 0[]const std.json.Value: 0Testing
zig buildzig build test --summary allzig build run-generatezig test generated/compile_generated.zigzig build-exe generated/main.zig -fno-emit-bin../openai.zig/.zig-cache/openai.openapi.jsoncd ../openai.zig && zig build generate && zig build test && zig buildResponseStreamEventandInputParamunions from JSONcd ../instructor.zig && zig build test && zig build examples