Skip to content

fix: prevent unknown character corruption in translations - #68

Merged
Starllordz merged 2 commits into
1.3.4from
fix/unknown-characters
May 25, 2026
Merged

Starllordz merged 2 commits into
1.3.4from
fix/unknown-characters

Conversation

@Starllordz

Copy link
Copy Markdown
Collaborator

Summary

Fixes the long-standing unknown character corruption observed across translations — both the visible ? characters in plain text (e.g. з??ду instead of згоду in Ukrainian) and the rarer U+FFFD corruption in Cyrillic / Greek / CJK / Arabic targets.

Two distinct root causes:

  1. Lara HTML auto-detection. TextBlock[] input with no contentType was auto-detected by the API as HTML-flavored, which mangled plain text containing curly apostrophes, dashes, and multi-byte characters into ?.
  2. UTF-8 streaming bug in @translated/lara. The SDK calls chunk.toString() per TCP data event, so multi-byte UTF-8 characters that straddle a chunk boundary lose bytes and decode to U+FFFD. Becomes visible at batch sizes ≥ a few keys because the response is large enough to span multiple TCP chunks. Reproduced deterministically in sdk-utf8-streaming.repro.test.ts.

What changed

  • Per-parser contentType declaration. New getContentType() on the Parser interface, exposed through ParserFactory. Default is text/plain for value-based formats (JSON, PO, TS, Vue, Markdown, Xcode .strings / .stringsdict / .xcstrings, TXT); text/html for Android XML where inline markup is part of the format.
  • Per-value detection and batch splitting. A small hasHtmlMarkup heuristic upgrades any individual value containing <a>, <b>, <br/>, etc. to text/html. Batches are grouped by detected content type across the whole file, so a JSON mixing "Hello" and "Click <a>here</a>" is sent as two API calls (one per content type) instead of one corrupting call.
  • U+FFFD retry guard. After each translation result the engine checks for �. Any affected task is retried up to 3 times as a solo call (tiny response, almost never spans a chunk). If all 3 retries still come back corrupted, the run exits 1 with a short "Translation failed for some keys. Please retry." message — no SDK internals leak into user-facing output.
  • Version bump: 1.3.3 → 1.3.4 in package.json and the README badge.

Tests

  • 742 tests pass (was 720).
  • New parameterized integration test (content-type-routing.integration.test.ts) verifies that each of the 10 supported file formats routes plain-text values to text/plain and HTML-bearing values to text/html.
  • New utf8-retry.integration.test.ts drives the retry guard end-to-end: one test shows a corrupted batch entry is recovered by the solo retry; the other shows the neutral failure message after three exhausted retries.
  • New sdk-utf8-streaming.repro.test.ts reproduces the upstream SDK bug deterministically (no network) by driving the real Translator against a local HTTP server that splits the response between the two UTF-8 bytes of и. Includes a StringDecoder-based test showing the one-line upstream fix.
  • Existing parser/integration tests updated with getContentType() assertions.

Test plan

  • pnpm install && pnpm test — all 742 tests pass
  • pnpm run build && pnpm run lint && pnpm run format:check — clean
  • Manual: translate a long English source with curly apostrophes (e.g. the ToS paragraph in the reporter's screenshots) to uk and bg. Confirm no ? appears mid-word and no тез?? / тез�� in the output.
  • Manual: translate a JSON value containing Click <a href="/x">here</a> and verify the tags survive the round trip.

Follow-ups

The U+FFFD corruption is fundamentally an upstream issue in @translated/lara's node-client.js:172 (buffer += chunk.toString() per data event — should be StringDecoder('utf8').write(chunk)). The retry guard mitigates user impact for now; a one-line upstream PR would let us delete the guard and the repro test. sdk-utf8-streaming.repro.test.ts is suitable to attach to that upstream report.

## Changed
- Set contentType explicitly per parser so Lara no longer auto-detects
  TextBlock[] as HTML and replaces non-ASCII chars with literal `?`
- Engine splits each batch by detected content type so values containing
  inline HTML are sent as text/html and plain values as text/plain
- Retry U+FFFD-corrupted translations up to 3 times as solo calls before
  failing with a neutral "please retry" message
- Bump version to 1.3.4

## New
- contentType utility (hasHtmlMarkup / resolveContentType) and per-parser
  getContentType() exposed through ParserFactory
- Parameterized integration tests covering content-type routing for all
  10 supported file formats
- Deterministic test reproducing the upstream UTF-8 streaming bug in
  @translated/lara, plus integration coverage for the retry guard

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses translation output corruption by explicitly controlling Lara contentType per parsed format/value and adding a mitigation for an upstream UTF-8 chunk-decoding bug that can introduce U+FFFD (�) in streaming responses.

Changes:

  • Add parser-level getContentType() defaults (plain-text formats → text/plain, Android XML → text/html) and route values via a per-value HTML-markup heuristic.
  • Update TranslationEngine to split batch requests by resolved contentType and retry any �-corrupted results as solo calls (up to 3 attempts), failing with a neutral message if still corrupted.
  • Add/extend unit + integration coverage for content-type routing and UTF-8 corruption reproduction/mitigation; bump version to 1.3.4.

Reviewed changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/utils/contentType.ts Adds HTML-markup detection + per-value contentType resolution helper.
src/interface/parser.ts Extends Parser interface with getContentType().
src/parsers/parser.factory.ts Exposes getContentType() through the factory facade.
src/parsers/json.parser.ts Declares default contentType as text/plain.
src/parsers/po.parser.ts Declares default contentType as text/plain.
src/parsers/ts.parser.ts Declares default contentType as text/plain.
src/parsers/vue.parser.ts Declares default contentType as text/plain.
src/parsers/markdown.parser.ts Declares default contentType as text/plain.
src/parsers/txt.parser.ts Declares default contentType as text/plain.
src/parsers/xcode-strings.parser.ts Declares default contentType as text/plain.
src/parsers/xcode-stringsdict.parser.ts Declares default contentType as text/plain.
src/parsers/xcode-xcstrings.parser.ts Declares default contentType as text/plain.
src/parsers/android-xml.parser.ts Declares default contentType as text/html for Android resources.
src/modules/translation/translation.engine.ts Implements content-type routing/splitting and U+FFFD retry guard; passes contentType into Lara options.
src/messages/messages.ts Adds neutral failure message for exhausted corruption retries.
src/tests/utils/contentType.test.ts Unit tests for markup detection and content-type resolution.
src/tests/sdk-utf8-streaming.repro.test.ts Deterministically reproduces upstream UTF-8 streaming bug and demonstrates StringDecoder fix.
src/tests/parsers/json.parser.test.ts Asserts getContentType() returns text/plain.
src/tests/parsers/po.parser.test.ts Asserts getContentType() returns text/plain.
src/tests/parsers/ts.parser.test.ts Asserts getContentType() returns text/plain.
src/tests/parsers/vue.parser.test.ts Asserts getContentType() returns text/plain.
src/tests/parsers/markdown.parser.test.ts Asserts getContentType() returns text/plain.
src/tests/parsers/txt.parser.test.ts Asserts getContentType() returns text/plain.
src/tests/parsers/xcode-strings.parser.test.ts Asserts getContentType() returns text/plain.
src/tests/parsers/xcode-stringsdict.parser.test.ts Asserts getContentType() returns text/plain.
src/tests/parsers/xcode-xcstrings.parser.test.ts Asserts getContentType() returns text/plain.
src/tests/parsers/android-xml.parser.test.ts Asserts getContentType() returns text/html.
src/tests/parsers/parser.factory.test.ts Validates ParserFactory.getContentType() per supported extension.
src/tests/integration/content-type-routing.integration.test.ts Verifies per-format routing of plain vs HTML-bearing values to correct contentType.
src/tests/integration/utf8-retry.integration.test.ts End-to-end tests for U+FFFD detection, solo retry recovery, and neutral failure after retries.
src/tests/integration/batch-translation.integration.test.ts Regression tests for explicit contentType and mixed plain+HTML batch splitting.
src/tests/integration/android-xml.integration.test.ts Asserts Android XML translation uses contentType=text/html.
package.json Bumps package version to 1.3.4.
README.md Updates version badge to 1.3.4.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/__tests__/utils/contentType.test.ts Outdated
## Changed
- Replace relative `../../utils/contentType.js` import with `#utils/contentType.js`
  to match the convention used by sibling utility tests (e.g. entities.test.ts).
@Starllordz
Starllordz merged commit b99185b into 1.3.4 May 25, 2026
Starllordz added a commit that referenced this pull request May 26, 2026
* fix: prevent unknown character corruption in translations (#68)

* fix: prevent unknown character corruption in translations

## Changed
- Set contentType explicitly per parser so Lara no longer auto-detects
  TextBlock[] as HTML and replaces non-ASCII chars with literal `?`
- Engine splits each batch by detected content type so values containing
  inline HTML are sent as text/html and plain values as text/plain
- Retry U+FFFD-corrupted translations up to 3 times as solo calls before
  failing with a neutral "please retry" message
- Bump version to 1.3.4

## New
- contentType utility (hasHtmlMarkup / resolveContentType) and per-parser
  getContentType() exposed through ParserFactory
- Parameterized integration tests covering content-type routing for all
  10 supported file formats
- Deterministic test reproducing the upstream UTF-8 streaming bug in
  @translated/lara, plus integration coverage for the retry guard

* chore: use #utils path alias in contentType test imports

## Changed
- Replace relative `../../utils/contentType.js` import with `#utils/contentType.js`
  to match the convention used by sibling utility tests (e.g. entities.test.ts).

* chore: upgrade glob and patch brace-expansion CVE (#69)

## Changed
- Bump `glob` from `^11.1.0` to `^13.0.6` to remove deprecation warning at install
- Override transitive `brace-expansion` in vulnerable range `>=5.0.0 <5.0.6` to `^5.0.6` (CVE-2026-45149, GHSA-jxxr-4gwj-5jf2)
@Starllordz
Starllordz deleted the fix/unknown-characters branch June 10, 2026 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants