Conversation
Co-Authored-By: judah <jsklan.development@gmail.com>
…erator Co-Authored-By: judah <jsklan.development@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Co-Authored-By: judah <jsklan.development@gmail.com>
There was a problem hiding this comment.
Devin Review found 1 potential issue.
🐛 1 issue in files not directly in the diff
🐛 Websocket path parameters not updated to use encode_path_param (generators/python/src/fern_python/generators/sdk/client_generator/websocket_connect_method_generator.py:542)
The PR's fix for boolean path parameter encoding was applied to endpoint_function_generator.py (line 1019) but the identical pattern in websocket_connect_method_generator.py:542 still uses jsonable_encoder. This means websocket endpoints with boolean path parameters will still serialize as Python-style True/False instead of lowercase true/false. The two methods (_get_path_for_endpoint and _get_path_for_websocket) are structurally identical, and the transformation was only applied to one of them.
View 2 additional findings in Devin Review.
Co-Authored-By: judah <jsklan.development@gmail.com>
|
Addressed the Devin Review finding — updated |
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
Description
Fix boolean path parameters in the Python SDK being serialized as Python-style
True/Falseinstead of JSON-styletrue/falsein URL paths. This causes WireMock test failures and incorrect API calls when boolean values appear in path segments.Root cause: The generator used
jsonable_encoder()for path parameter interpolation. Sinceboolis a subclass ofintin Python,jsonable_encoderreturns booleans as-is, and Python's defaultstr()producesTrue/Falsewhen interpolated into f-strings.Changes Made
encode_path_param()utility function injsonable_encoder.pythat explicitly handles booleans with lowercasetrue/false, and delegates tojsonable_encoder+str()for all other typesencode_path_paramas an exported core utility incore_utilities.pyendpoint_function_generator.pyto useencode_path_paraminstead ofjsonable_encoderfor path parameter interpolationwebsocket_connect_method_generator.pyto also useencode_path_param(same pattern as endpoint generator — flagged by Devin Review)getWithBooleanPathendpoint to the exhaustive test fixture to cover boolean path parametersiranddynamic-snippets)Important Review Notes
str(jsonable_encoder(obj))fallback inencode_path_parammirrors the existing implicit stringification that happens when values are interpolated into f-strings, so non-boolean types should behave identically to before.seed-test-results (python-sdk)CI check passes (generated code compiles and tests pass), but the actual seed snapshot files underseed/python-sdk/exhaustive/have not been committed in this PR. A reviewer may want to runpnpm seed:local test --generator python-sdk --fixture exhaustivelocally and commit the updated snapshots, or confirm CI's passing result is sufficient.test-eteCI check failure is unrelated — it's a timeout ingenerate-with-settings.test.ts > dependencies-based apiand is not marked as required.Testing
poetry run pre-commit run -a)pnpm formatpassespnpm seed:local run --generator python-sdk --path <customer-api-path>— confirmed WireMock error:Path parameter: ignoreEmpty = true | True <<<<< Path parameter does not matchFor reviewer
endpoint_function_generator.pyandwebsocket_connect_method_generator.pythat interpolate path parameters and may need the same fix?encode_path_paramhandles edge cases (e.g.,None, enums, nested types) — thestr(jsonable_encoder(obj))fallback should cover these, but worth confirmingseed/python-sdk/exhaustive/need to be committed (CI seed tests pass without them)Link to Devin session: https://app.devin.ai/sessions/25a25e3ba67447f59aa5d11486e44726
Requested by: @jsklan