fix: send non_splitting_tags and ignore_tags as arrays in JSON payloads#92
Merged
usox merged 1 commit intoSC-Networks:masterfrom Apr 8, 2025
Merged
fix: send non_splitting_tags and ignore_tags as arrays in JSON payloads#92usox merged 1 commit intoSC-Networks:masterfrom
usox merged 1 commit intoSC-Networks:masterfrom
Conversation
Note: other request handlers (e.g., DeepBatchTranslationRequestHandler) and tests may still contain similar issues. This commit does not address those.
Member
|
Hi @modrictin Thank you for your contribution and the detailed description. This is looking good so far, we will have a look at the remaining payloads and will cover that within #93 |
usox
approved these changes
Apr 8, 2025
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.
🛠️ What this fixes:
We got a Http error occurred (400) error when migrating from 2.x to 4.0.
This PR corrects how
non_splitting_tags,ignore_tagsare handled when building the DeepL translation request body.Previously, these were passed as comma-separated strings via:
This worked when the request body was encoded as
application/x-www-form-urlencoded, but the current implementation builds a JSON request body using a stream and sets theContent-Typetoapplication/json.✅ What this PR does:
non_splitting_tagsandignore_tagsas proper arrays in the JSON body (instead of comma-separated strings)application/jsonpayloadsHttp error occurred (400)when settingnon_splitting_tagsorignore_tags🧪 Verified behavior:
We confirmed this fix by sending the following test input:
With:
✅ Result (correct behavior):
Without
non_splitting_tags, DeepL treated the tags as sentence boundaries, leading to split context and degraded translation quality.The same principle applies to
ignore_tags, which must also be passed as an array when using JSON payloads.This PR only addresses the issue in the
DeeplTranslationRequestHandler.However, the same incorrect handling of list-based parameters (via
implode(...)) appears to be present in at least:DeepBatchTranslationRequestHandlerThere may be other request handlers or tests affected as well — I haven’t reviewed all of them.
🚧 This PR is not a full fix across the entire package. Contributions are welcome to complete this.
P.S. 'tag_handling' can be affected as well, but I don't think so.
✅ Summary:
This update brings the client in line with DeepL API expectations for JSON requests. It resolves a backend-breaking change and ensures future-proof compatibility with
non_splitting_tags,ignore_tags, and other list-style fields.