Skip to content

fix(runtime): serialize parameters by declared style - #192

Merged
samzong merged 2 commits into
mainfrom
fix/parameter-serialization
Oct 3, 2026
Merged

samzong merged 2 commits into
mainfrom
fix/parameter-serialization

Conversation

@samzong

@samzong samzong commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Closes #117.

  • OpenAPI 3 style, explode, and allowReserved are preserved on parameters and honored on the wire: path simple/label/matrix, query form/spaceDelimited/pipeDelimited with and without explode, and allowReserved for query values.
  • Cookie parameters are supported. They are appended after authentication cookies and never replace them; a cookie flag that collides with another parameter flag becomes cookie-<flag>.
  • Path array parameters become []string flags. Swagger 2 collectionFormat maps to the equivalent style for query and path arrays; header arrays keep the existing joined-string behavior. tsv and unsupported path formats fail codegen with an error naming the original collectionFormat.
  • Unsupported shapes (object parameters, deepObject, and similar) fail codegen after overlays are applied, so an overlay ignore: true on the operation is the escape hatch. The reported error is deterministic and matched by operation ID, method, and path.
  • Pagination keeps the raw query when it updates the page parameter, so later pages use the same encoding as the first.
  • Dry-run shows a cookie pair only when it exactly equals a value produced by a non-sensitive cookie parameter; authentication cookies and sensitive names (session, sid, csrf, and existing sensitive header/query names) stay redacted. Debug query redaction also covers ;-separated segments without rewriting separators.

Verification

  • make check passed.
  • go test -race ./pkg/runtime/... ./internal/codegen/... ./internal/lathecmd/... passed.
  • examples/richapi: lathe codegen -cache fixtures, build, __lathe verify --json returned ok: true. Dry-run of an array query showed ?limit=20&roles=a+b,c%2Fd and Cookie: tenant=x%20y.
  • An independent review reproduced wire output against an httptest server for path simple/label/matrix with explode, query form/space/pipe with explode, allowReserved, cookie escaping, auth cookie ordering, cursor and offset pagination from page 2 onward, and InvokeOperation; dry-run URLs match the wire. Its findings (cookie flag collisions, auth cookie leakage in dry-run, weak cookie sensitivity, Swagger header ssv/pipes, duplicate operation IDs with ignore, nondeterministic errors, ; query redaction) are fixed and were re-verified.

Not verified: allowReserved values that already contain percent-encoded triplets are encoded again (%41 becomes %2541); this is out of scope. Live services with cookie parameters were not exercised.

Compatibility

  • runtime.SchemaVersion 21 and runtime.CatalogSchemaVersion 28; regenerate downstream CLIs.
  • Swagger 2 array query parameters without collectionFormat now default to csv (k=a,b) as the specification requires, instead of repeated keys.
  • Specs with unsupported parameter shapes that previously generated now fail codegen until the operation is ignored in an overlay.
  • Path array parameters change from string to []string; overlay shortcut presets or context bindings that target them fail codegen.

Checklist

  • Tests or focused verification cover the changed surface.
  • User-facing behavior changes are documented.
  • Generated output under internal/generated/, .cache/, and ad-hoc skills/<cli-name>/ directories is not committed.
  • Commits are signed off when this is ready to merge.

Signed-off-by: samzong <samzong.lu@gmail.com>
@ghfind-review ghfind-review Bot added the review: top ghfind author score; see https://ghfind.com label Oct 3, 2026
@codspeed

codspeed Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will degrade performance by 43.84%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 21 regressed benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ large 1.2 ms 2.2 ms -47.43%
❌ BenchmarkBuildFlat 1 ms 2 ms -47.35%
❌ yaml 2.7 ms 5 ms -47.23%
❌ large 871.7 µs 1,617.1 µs -46.1%
❌ small 64.8 µs 118.6 µs -45.34%
❌ yaml-large 8.2 ms 14.9 ms -45.19%
❌ BenchmarkParseNormalize 3.9 ms 7.2 ms -45.08%
❌ miss 1.9 ms 3.5 ms -44.96%
❌ hit 2 ms 3.6 ms -44.78%
❌ BenchmarkFormatTableInferredColumns 608.4 µs 1,092.6 µs -44.32%
❌ BenchmarkCatalogJSON 1.6 ms 2.8 ms -44.04%
❌ small 77.6 µs 138.2 µs -43.82%
❌ json 730 µs 1,291.4 µs -43.47%
❌ large 891.5 µs 1,576.7 µs -43.46%
❌ json-small 142.1 µs 246.2 µs -42.27%
❌ BenchmarkFindCatalogCommand 4.6 µs 7.9 µs -41.79%
❌ small 81.1 µs 138.5 µs -41.4%
❌ table 414.1 µs 706.2 µs -41.36%
❌ large 1.3 ms 2.2 ms -40.76%
❌ small 103.8 µs 174 µs -40.33%
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing fix/parameter-serialization (ed23ca7) with main (34a1e47)

Open in CodSpeed

…zation

Signed-off-by: samzong <samzong.lu@gmail.com>

# Conflicts:
#	docs/cli-usage.md
#	docs/contracts.md
#	pkg/runtime/catalog_projection_test.go
#	pkg/runtime/catalog_schema.go
#	pkg/runtime/spec.go
@samzong
samzong marked this pull request as ready for review October 3, 2026 19:21
@samzong
samzong merged commit acb5b7f into main Oct 3, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review: top ghfind author score; see https://ghfind.com

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(openapi): honor parameter serialization in generated requests

1 participant