fix(client): serialize list query params with empty brackets - #69
Open
pravinireri wants to merge 1 commit into
Open
fix(client): serialize list query params with empty brackets#69pravinireri wants to merge 1 commit into
pravinireri wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the SDK’s query-string serialization to encode list-valued query parameters as repeated empty-bracket keys (types[]=a&types[]=b) instead of indexed keys (types[0]=a&types[1]=b), aligning with the API’s expected format and fixing the reported 400s for list query params.
Changes:
- Added
Util::encodeQuery()to serialize query params with RFC3986 escaping and list values as repeatedkey[]=pairs. - Switched both
Util::joinUri()andRequestTransformer::build()to use the shared query serializer (removing reliance onhttp_build_query()for these paths). - Expanded test coverage to assert correct list serialization and preservation through request rebuilding and real client calls.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Core/UtilTest.php | Updates existing expectations and adds focused tests for empty-bracket list encoding and encoding invariants. |
| tests/Core/RequestTransformerTest.php | Adds regression test to ensure list query params keep empty brackets after transformer rebuild. |
| tests/ClientTest.php | Adds end-to-end assertions that wire queries use empty brackets for list params (including extraQueryParams). |
| src/Core/Util.php | Introduces encodeQuery() / encodeQueryPart() and switches joinUri() to the new serializer. |
| src/Core/RequestTransformer.php | Switches request rebuild query serialization to Util::encodeQuery(). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
http_build_query() emits numerically indexed keys for PHP lists (types[0]=a&types[1]=b), which the API rejects with a 400; it expects repeated empty-bracket pairs (types[]=a&types[]=b). Encode query params with a helper that recognizes lists via array_is_list() and emits key[]=value pairs, preserving associative keys, scalar encoding (RFC3986), ordering, and merge behavior. Applies to both Util::joinUri and RequestTransformer::build so rewritten Bedrock/Vertex requests keep the same encoding. Fixes anthropics#67
pravinireri
force-pushed
the
fix-list-query-params
branch
from
August 17, 2026 19:19
3b40cad to
85d0ffa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
types[]=a&types[]=b) instead of numeric indices (types[0]=a&types[1]=b).Util::joinUri()andRequestTransformer::build().Tests
./vendor/bin/pest tests/Core/UtilTest.php tests/Core/RequestTransformerTest.php tests/ClientTest.php./vendor/bin/pestagainst the Steady mock server — 187 passed./vendor/bin/phpstan analyse --memory-limit=12G— no errorsFixes #67