Merge pull request #42 from gpu-cli/fix/float-precision-and-param-enum-varnames

fix: float precision and parameter-level x-enum-varnames

Author: lightsofapollo

.beads/issues.jsonl

@@ -17,8 +17,22 @@ when correcting output that was wrong or incomplete on the wire.
   SSE variant names and require a runtime status for wildcard/default variants.
   This is a source-breaking correction for existing server trait implementations.
 
+### Changed
+
+- `format: float` now maps to `f64` instead of `f32`. JSON carries no binary32,
+  so the declared format describes the server's storage rather than the
+  transport: a value sent as `0.03` survives in `f64` but becomes
+  `0.029999999329447746` through `f32`, which matters when the field is money.
+  Set `float_precision = "f32"` under `[generator.types]` to map strictly by
+  declared format. `--types-conservative` keeps the literal `f32` mapping.
+
 ### Fixed
 
+- Parameter-level inline enums honor `x-enum-varnames`. Schema-level enums
+  already did, so the same enum produced different Rust variant names depending
+  on whether it lived in `components.schemas` or on a parameter. A varnames
+  array whose length disagrees with `enum` is ignored rather than applied to a
+  prefix.
 - Properties that are both `required` and nullable via OpenAPI 3.1's
   `type: ["X", "null"]` now generate `Option<T>` instead of a bare `T`, in plain
   object schemas and in `allOf`-composed ones alike. Previously such a client

CHANGELOG.md

@@ -642,11 +642,18 @@ binary    = "bytes"                      # bytes   (default) | vec_u8 | string
 uuid      = "uuid"                       # uuid    (default) | string
 byte      = "base64"                     # base64 (default) | base64_url_unpadded | vec_u8 | string
 unsigned  = true                         # uint32/uint64 -> u32/u64
+float_precision = "f64"                  # f64 (default) | f32 — see below
 
 [generator.types.shape]
 additional_properties_typed = true
 ```
 
+**`format: float` maps to `f64`, not `f32`.** JSON carries no binary32, so the
+declared format describes the server's storage rather than the transport. A
+price sent on the wire as `0.03` parses losslessly into `f64`, but through
+`f32` it becomes `0.029999999329447746` — a real hazard when the field is
+money. Set `float_precision = "f32"` to map strictly by declared format.
+
 ## Testing
 
 ```bash

README.md

@@ -17,6 +17,10 @@
 #   SPEC_COMPILE_LIMIT=N    process only the first N alphabetically-sorted specs
 #   SPEC_COMPILE_PARSE_ONLY=1  skip cargo check; only verify the generator
 #                              parses+emits without errors. Faster.
+#   SPEC_COMPILE_FORCE_CHECK=1 also cargo check the specs in
+#                              GENERATE_ONLY_SPECS (see below), which are
+#                              skipped by default because their generated
+#                              crate exceeds CI runner memory.
 #   SPEC_COMPILE_TARGET_DIR=path  shared cargo target dir for the scratch
 #                              workspace (default tmp/spec-compile-target).
 #                              Dependency artifacts (reqwest, chrono, …)
@@ -78,11 +82,40 @@ fi
 echo "[spec-compile] running ${#SPECS[@]} spec(s)"
 echo
 
+# Specs whose generated crate is too large to `cargo check` inside a standard
+# CI runner. They are still generated (which catches the majority of generator
+# defects); only the compile step is skipped, and the summary reports them
+# separately so a green run is never mistaken for full verification.
+#
+# Set SPEC_COMPILE_FORCE_CHECK=1 to check them anyway on a machine with the
+# headroom. Measured peaks, `cargo check`, single rustc process:
+#   microsoft-graph  2.4M lines generated  ~14.3 GB RSS  (16,153 operations)
+# A GitHub-hosted ubuntu-latest runner has 16 GB total, so it is killed with
+# SIGTERM partway through. Raising cargo parallelism does not help — the memory
+# is one rustc type-checking one crate.
+GENERATE_ONLY_SPECS=("microsoft-graph")
+
+is_generate_only() {
+  [ "${SPEC_COMPILE_FORCE_CHECK:-}" = "1" ] && return 1
+  for entry in "${GENERATE_ONLY_SPECS[@]}"; do
+    [ "$entry" = "$1" ] && return 0
+  done
+  return 1
+}
+
+generate_only_reason() {
+  case "$1" in
+    microsoft-graph) echo "~14.3 GB RSS, exceeds CI runner memory" ;;
+    *) echo "exceeds CI runner resources" ;;
+  esac
+}
+
 # ---- Phase 1: generate a scratch crate per spec -------------------------
 passed=()
 failed_gen=()
 failed_check=()
 skipped=()
+generate_only=()
 gen_ok=()
 for entry in "${SPECS[@]}"; do
   IFS='|' read -r name spec_path <<<"$entry"
@@ -163,6 +196,11 @@ elif [ ${#gen_ok[@]} -gt 0 ]; then
   echo
   echo "[spec-compile] cargo check (${#gen_ok[@]} isolated manifest(s))..."
   for name in "${gen_ok[@]}"; do
+    if is_generate_only "$name"; then
+      printf "%-30s GEN-ONLY (cargo check skipped: %s)\n" "$name" "$(generate_only_reason "$name")"
+      generate_only+=("$name")
+      continue
+    fi
     log="$ROOT/$name/check.log"
     if ( cd "$ROOT/$name" && CARGO_TARGET_DIR="$SCRATCH_TARGET" cargo check $OFFLINE ) >"$log" 2>&1; then
       printf "%-30s PASS\n" "$name"
@@ -177,12 +215,21 @@ elif [ ${#gen_ok[@]} -gt 0 ]; then
 fi
 
 echo
-echo "[spec-compile] summary: ${#passed[@]} passed, ${#failed_gen[@]} gen-failed, ${#failed_check[@]} check-failed, ${#skipped[@]} skipped"
+echo "[spec-compile] summary: ${#passed[@]} passed, ${#failed_gen[@]} gen-failed, ${#failed_check[@]} check-failed, ${#generate_only[@]} generate-only, ${#skipped[@]} skipped"
 [ ${#failed_gen[@]}   -gt 0 ] && echo "  gen-fail:   ${failed_gen[*]}"
 [ ${#failed_check[@]} -gt 0 ] && echo "  check-fail: ${failed_check[*]}"
 [ ${#skipped[@]}      -gt 0 ] && echo "  skipped:    ${skipped[*]}"
+if [ ${#generate_only[@]} -gt 0 ]; then
+  echo "  generate-only (NOT compile-verified): ${generate_only[*]}"
+  echo "  ^ these generated cleanly but were never compiled. Run them locally"
+  echo "    on a machine with enough RAM: scripts/spec-compile.sh ${generate_only[*]}"
+fi
 
 if [ ${#failed_gen[@]} -gt 0 ] || [ ${#failed_check[@]} -gt 0 ]; then
   exit 1
 fi
-echo "[spec-compile] ✅ all specs compiled cleanly"
+if [ ${#generate_only[@]} -gt 0 ]; then
+  echo "[spec-compile] ✅ ${#passed[@]} spec(s) compiled cleanly; ${#generate_only[@]} generated but not compiled"
+else
+  echo "[spec-compile] ✅ all specs compiled cleanly"
+fi

scripts/spec-compile.sh

@@ -447,6 +447,13 @@ pub struct ParameterInfo {
     /// See issue #10 follow-up.
     #[serde(skip_serializing_if = "Option::is_none")]
     pub enum_values: Option<Vec<String>>,
+    /// `x-enum-varnames` declared on the parameter's inline enum schema, when
+    /// present and the same length as `enum_values`. Schema-level enums already
+    /// honor this vendor extension through `SchemaAnalysis::enum_extensions`;
+    /// parameter enums are inline and have no analyzed-schema name to key on,
+    /// so their names ride along here instead.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub enum_varnames: Option<Vec<String>>,
     /// Disambiguated Rust ident assigned by the analyzer at the operation
     /// scope. When two parameters in the same operation sanitize to the same
     /// snake_case name (e.g. `exclude_ids` + `exclude-ids` in vercel,
@@ -4734,6 +4741,7 @@ impl SchemaAnalyzer {
                 rust_type: "String".to_string(),
                 description: None,
                 enum_values: None,
+                enum_varnames: None,
                 rust_ident: None,
                 query_serialization: None,
                 validation_schema: None,
@@ -4986,6 +4994,7 @@ impl SchemaAnalyzer {
         let mut rust_type = "String".to_string();
         let mut schema_ref = None;
         let mut enum_values: Option<Vec<String>> = None;
+        let mut enum_varnames: Option<Vec<String>> = None;
         let mut query_serialization: Option<QuerySerialization> = None;
 
         // OAS 3.x style/explode resolution for `in: query`. Defaults are
@@ -5102,6 +5111,21 @@ impl SchemaAnalyzer {
                                 let op_pascal = operation_id.replace('.', "_").to_pascal_case();
                                 let param_pascal = name.to_pascal_case();
                                 rust_type = format!("{op_pascal}{param_pascal}");
+                                // Honor `x-enum-varnames` here the same way
+                                // schema-level enums do. A mismatched length is
+                                // ambiguous about which value each name refers
+                                // to, so drop it rather than guess.
+                                enum_varnames = details
+                                    .extra
+                                    .get("x-enum-varnames")
+                                    .and_then(Value::as_array)
+                                    .map(|raw| {
+                                        raw.iter()
+                                            .filter_map(Value::as_str)
+                                            .map(str::to_owned)
+                                            .collect::<Vec<_>>()
+                                    })
+                                    .filter(|names| names.len() == values.len());
                                 enum_values = Some(values);
                             }
                         }
@@ -5167,6 +5191,7 @@ impl SchemaAnalyzer {
             rust_type,
             description: param.description.clone(),
             enum_values,
+            enum_varnames,
             rust_ident: None,
             query_serialization,
             validation_schema,

src/analysis.rs

@@ -1110,10 +1110,23 @@ impl CodeGenerator {
         // while keeping each `serde(rename)` pointing at the original
         // wire string.
         let mut used: std::collections::HashSet<String> = std::collections::HashSet::new();
+        // `x-enum-varnames` wins over the naming heuristic when the spec
+        // supplies it — the whole point of the extension is that the author
+        // knows better than a transformation of the wire string. Schema-level
+        // enums already honored it; parameter enums did not, so the same spec
+        // produced different variant names depending on where its enum lived.
+        // Suffix disambiguation still applies, since nothing stops a spec from
+        // declaring two names that collide once converted to an identifier.
         let variant_names: Vec<String> = values
             .iter()
-            .map(|value| {
-                let base = self.to_rust_enum_variant(value);
+            .enumerate()
+            .map(|(index, value)| {
+                let base = param
+                    .enum_varnames
+                    .as_ref()
+                    .and_then(|names| names.get(index))
+                    .map(|name| self.to_rust_enum_variant(name))
+                    .unwrap_or_else(|| self.to_rust_enum_variant(value));
                 let mut chosen = base.clone();
                 let mut suffix = 2;
                 while !used.insert(chosen.clone()) {

src/client_generator.rs

@@ -1091,6 +1091,15 @@ impl CodeGenerator {
 
                 /// Decode the generated RFC 9457 validation-problem profile
                 /// without replacing a documented per-operation error in `typed`.
+                ///
+                /// Returns `None` unless the response's `Content-Type` is
+                /// `application/problem+json`, which is how RFC 9457 identifies
+                /// a problem document. Most third-party APIs return their
+                /// errors as plain `application/json`, so this yields `None`
+                /// against them by design — use `typed` for a documented
+                /// per-operation error body, or `body` for the raw payload.
+                /// Servers generated by this tool always emit the problem
+                /// media type, so this succeeds against them.
                 pub fn problem_details(
                     &self,
                 ) -> Option<openapi_to_rust_problem::ProblemDetails> {

src/generator.rs

@@ -916,6 +916,7 @@ mod tests {
                 rust_type: "String".to_string(),
                 description: None,
                 enum_values: None,
+                enum_varnames: None,
                 rust_ident: None,
                 query_serialization: None,
                 validation_schema: Some(json!({"$ref": "#/components/schemas/Payload"})),
@@ -968,6 +969,7 @@ mod tests {
                 rust_type: "serde_json::Value".to_string(),
                 description: None,
                 enum_values: None,
+                enum_varnames: None,
                 rust_ident: None,
                 query_serialization: None,
                 validation_schema: Some(json!({"const": literal})),
@@ -1010,6 +1012,7 @@ mod tests {
                 rust_type: "String".to_string(),
                 description: None,
                 enum_values: None,
+                enum_varnames: None,
                 rust_ident: None,
                 query_serialization: None,
                 validation_schema: Some(json!({"type": "string", "maxLength": 4})),

src/server/validation.rs

@@ -0,0 +1,14 @@
+---
+source: src/test_helpers.rs
+expression: "&generated_code"
+---
+//! Generated types from OpenAPI specification
+//!
+//! This file contains all the generated types for the API.
+//! Do not edit manually - regenerate using the appropriate script.
+#![allow(clippy::large_enum_variant)]
+#![allow(clippy::format_in_format_args)]
+#![allow(clippy::let_unit_value)]
+#![allow(unreachable_patterns)]
+use serde::{Deserialize, Serialize};
+pub type ListInstancesResponse = String;

src/snapshots/openapi_to_rust__test_helpers__param_enum_no_varnames.snap

@@ -0,0 +1,14 @@
+---
+source: src/test_helpers.rs
+expression: "&generated_code"
+---
+//! Generated types from OpenAPI specification
+//!
+//! This file contains all the generated types for the API.
+//! Do not edit manually - regenerate using the appropriate script.
+#![allow(clippy::large_enum_variant)]
+#![allow(clippy::format_in_format_args)]
+#![allow(clippy::let_unit_value)]
+#![allow(unreachable_patterns)]
+use serde::{Deserialize, Serialize};
+pub type ListInstancesResponse = String;