Add ability to compose multi-value Machinery translations by mathjazz · Pull Request #4236 · mozilla/pontoon

mathjazz · 2026-06-18T14:25:25Z

codecov-commenter · 2026-06-18T14:28:16Z

Codecov Report

❌ Patch coverage is 82.32759% with 82 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.55%. Comparing base (bfe34e1) to head (2f38757).
⚠️ Report is 2 commits behind head on main.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

eemeli

Looked at the Python parts only thus far.

mathjazz · 2026-06-22T18:12:16Z

Thanks for the comments.

Please note that I've intentionally not flagged anyone for code review yet, because I'd like to first get feedback on the functionality. The code is still deployed to https://pontoon.allizom.org/.

mathjazz · 2026-07-01T09:53:25Z

Rebased and addressed comments.

flodolo · 2026-07-05T05:32:39Z

Could you rebase this? Also, did we remove this code from the dev branch? 🤔

eemeli · 2026-07-05T09:12:10Z

+    # Only multi-pattern messages — those with multiple properties and/or
+    # selector variants — have something to compose. A single-pattern message
+    # composes to the same string the per-leaf machinery already returns, so
+    # there is nothing extra to show.


This is not always true; if the source message contains a plural selector, it may have only one pattern, but require multiple patterns in the target locale.

I think the code works as you are describing, but the comment is confusing (also looking at the comment above), because it uses the term "pattern" differently than moz-l10n.

What do you think about replacing the term "pattern" here and in other places with "leaf"? A leaf essentially means the number of input fields shown in the editor.

What I mean is that if the entity.value is representing a Fluent message like

popup-warning-exceeded-message = { $popupCount -> *[other] { -brand-short-name } prevented this site from opening more than { $popupCount } pop-up windows. }

then languages like Slovenian will want to represent it with multiple patterns:

popup-warning-exceeded-message = { $popupCount -> [one] { -brand-short-name } je strani preprečil, da bi odprla več kot { $popupCount } pojavno okno. [two] { -brand-short-name } je strani preprečil, da bi odprla več kot { $popupCount } pojavni okni. [few] { -brand-short-name } je strani preprečil, da bi odprla več kot { $popupCount } pojavna okna. *[other] { -brand-short-name } je strani preprečil, da bi odprla več kot { $popupCount } pojavnih oken. }

even if the unused one variant is left out.

Good point. Let me address this.

eemeli · 2026-07-05T09:15:02Z

+    response = {
+        "original": entity.string,
+        "translation": translation,
+        "sources": sources_used,
+    }


Why is this API serialising the value+properties, and not returning them as data model values?

For consistency with the rest of the machinery API. Every machinery source returns { original, translation } as plain strings, and the Machinery panel sorts, dedups, renders, and copies them uniformly.

And, the frontend already parses full entry sources. On copy, distributeEntrySource calls `parseEntry(format, str)à to spread the leaves across the editor fields.

We should work towards (later, separately) transitioning the other machinery API endpoints to also using a data model representation for messages, as we need to get that in any case to validate that what we're getting e.g. from machine translation is valid.

That could be done as part of KR2: Refactor the TM entry representation around the message data model.

Eh, if you insist. But I do think the PR's current API is trying to fit a square peg in a round hole just because the round hole already exists.

eemeli · 2026-07-05T10:04:54Z

+  // Set for `/machinery-composed/` results, whose `original` and `translation`
+  // are full entry sources (Fluent attributes, MF2 variants) rather than plain
+  // strings — the Machinery panel renders these in a rich, multi-field view.
+  composed?: boolean;


Wouldn't it be simpler to define a new ComposedMachineryTranslation type, as it needs to carry multiple patterns instead of the single pattern of MachineryTranslations?

That would also make it easier to pass the value+properties in the API as JSON data model values, rather than serialising them on the server and re-parsing on the client.

eemeli

At least a couple of the front-end comments from the previous review still need to be addressed too.

Edit: Actually, just #4236 (comment)

eemeli · 2026-07-06T16:15:40Z

+    translation = body["translation"]
+    assert "[one]" in translation
+    assert "[two]" in translation
+    assert "[few]" in translation
+    assert "*[other]" in translation
+    assert translation.count("TM_popups") == 4
+    assert body["sources"] == ["translation-memory"]
+    assert body["quality"] == 100


This would be clearer as a single assert body == ... test.

eemeli · 2026-07-06T16:21:06Z

+def parse_db_string_to_json(
+    res_format: str,
+    source: str,
+) -> tuple[JsonMessage, dict[str, JsonMessage] | None]:
+    _, value, properties = parse_source_string_to_json(res_format, source)
+    return value, properties


This function should not exist, and we should just refactor the places where it's called to call parse_source_string_to_json instead.

eemeli · 2026-07-06T16:41:02Z

+            key, value, properties = parse_source_string_to_json(fmt, text)
+            entity = SimpleNamespace(
+                resource=resource,
+                string=text,
+                key=key,
+                value=value,
+                properties=properties,
+            )


This is the only non-test code that makes any use of the key value returned by parse_source_string_to_json, and the use it's put to is to set the corresponding value in the fake Entity that's constructed here, which is only used to set the id of the temporary Entry that's passed to set_accesskeys, which ignores it.

So we don't actually need to return it in the first case.

entity.key is also read by Pretranslation.serialize() (pretranslate.py:187)

eemeli · 2026-07-06T16:42:18Z

+                value=value,
+                properties=properties,
+            )
            pretranslation = get_pretranslation(entity=entity, locale=locale)


This needs another TODO/FIXME comment about refactoring so that we don't need to construct the fake Project, Resource, and Entity values to call it.

eemeli

Ah, sorry, looks like I missed some of the translate/ changes.

eemeli · 2026-07-06T16:52:49Z

+  let count = 1;
+  for (let i = 0; i < msg.sel.length; ++i) {
+    if (plurals.has(i)) {
+      count *= Math.max(1, pluralCategories);
+    } else {
+      // Non-plural selector (e.g. a gender): keep its distinct source keys.
+      const keys = new Set(
+        msg.alt.map((v) => {
+          const key = v.keys[i];
+          return typeof key === 'string' ? key : '*';
+        }),
+      );
+      count *= keys.size || 1;
+    }
+  }
+  return count;


This seems unnecessarily accurate. We don't actually care about the pattern count, we only care if it's more than one.

eemeli · 2026-07-06T16:57:27Z

+        specialFormats.has(format) &&
+        hasMultipleFields(
+          entity.value,
+          entity.properties,
+          locale.cldrPlurals.length,
+        );


Given the early exits in patternCount(), there's no reason to gate this on the format as well.

Suggested change

specialFormats.has(format) &&

hasMultipleFields(

entity.value,

entity.properties,

locale.cldrPlurals.length,

);

hasMultipleFields(

entity.value,

entity.properties,

locale.cldrPlurals.length,

);

flodolo · 2026-07-07T04:26:39Z

I think I just encountered a bug.

I have a translation for this string

perftools-presets-networking-with-logs-label = Networking with Logs

I have another string with different morphology, same text

profiler-popup-presets-networking-with-logs-label =
    .label = Networking with Logs

Pontoon tries to put the syntax in the field, and shows a 100% match with the syntax in MACHINERY.

For Fluent and MF2-handled formats (Android, Gettext, WebExt, Xcode, Xliff), the Machinery panel only matched on the first input field (via `getPlainMessage()`), so attributes and selector variants were never surfaced. This mirrors Pretranslation's complex string composition in Machinery, adding directly-pasteable composed suggestions alongside the existing results. More details: 1. Parameterize Pretranslation with mt_provider/mt_service_name/ mt_supported, and move entity-walking into `Pretranslation.walk_entity()` so Machinery can reuse the composition pipeline. 2. Add `/machinery-composed/` endpoint that walks the entity, looks up each value in TM, and falls back to the requested MT service for any remaining value. Returns the composed string + the actual mix of services used (TM badge + MT badge for hybrid results). 3. Frontend fires composed requests in parallel with the existing fetches when the entity format can have multiple values. Composed results dedupe through the existing `addResults()` merge.

on the entity having more than one translatable input, reusing the editor's field-counting logic. 2. Surface a quality badge. When every value is a 100% TM match, the composed string is a perfect TM match, so return quality 100 and pass it through to the panel. 3. Render composed (multi-value) suggestions as labeled fields, the same representation as the original string panel.

across all input fields, reusing the field-building logic that is already used by the History panel. The plain message is recorded as `machinery.translation` so that source attribution still matches the saved translation on submit.

string isn't suggested back to itself. The composed path didn't, so a composed TM result could be reconstructed from the entity's own translation after it was translated. Add an opt-in `exclude_entity` flag to Pretranslation that excludes the entity's own TM entries from per-value lookups, and enable it from the Machinery composed view. A value that can only be served by the entity's own translation then has no TM match, so a TM-only composition relying on it is no longer produced. Pretranslation behavior is unchanged.

Re-applying a composed Machinery suggestion (or restoring history) after editing a field took two clicks: the first did nothing. Typing updates only CodeMirror's internal doc and EditorResult, not EditorData.state.fields, so TranslationForm doesn't re-render and each EditField keeps a stale `defaultValue`. EditField re-syncs its document only in `useEffect(() => setValue(defaultValue), [defaultValue])`. distributeEntrySource builds new fields with placeholder handles while the on-screen editors stay bound, via their React key, to the previous fields' live handles. The re-applied value for the edited field equals its stale `defaultValue`, so the effect doesn't fire and the field isn't updated. A later re-render refreshes `defaultValue`, which is why the second click works. Push the distributed values straight into the live handles, matched by field id, the same way clearEditor does, so one click suffices.

The "Refine using AI" dropdown doesn't work on composed (multi-field/ plural) suggestions: the loader never shows, the refined result never updates the UI, and copying dumps the raw Fluent source into the first field. The backend /gpt-transform/ endpoint also refines a single string, so it can't preserve the entry structure (e.g. returns 2 plural forms instead of 4). Hide the dropdown for composed suggestions so they behave like a plain Google Translate source. Proper composed support is left as a follow-up.

When a suggestion combines multiple sources (e.g. GOOGLE TRANSLATE and TRANSLATION MEMORY), the source titles ran together with no separator.

…ntity.value and .properties represent a single-pattern or multi-pattern message.

value_from_string in pontoon.sync duplicated parse_db_string_to_json. Consolidate into a single parse_source_string_to_json in pontoon.translations (beside its inverse, serialize_for_db); parse_db_string_to_json now delegates to it, and the key-aware path picks up the MF2 catchall normalization the sync copy was missing.

…cialFormats, which is byte-for-byte identical

…ntity.properties with no parse. Also, document the source-side heuristic.

The composed-machinery gate counted source patterns, so an en-US message with only `*[other]` was treated as single- pattern and skipped — even for locales like Slovenian that need one/two/few/other. Count target patterns instead: the backend reuses the plural expansion walk_entity() already performs, and the frontend's hasMultipleFields expands plural selectors to the locale's CLDR categories. A single-variant source now composes to a full multi-pattern suggestion for the target locale.

…ntry) has just one translatable leaf, so its composed suggestion would merely duplicate the per-leaf TM/MT match. Both the frontend and backend pattern counters, however, counted the empty value as a leaf, so such an entity looked multi-pattern and a redundant composed suggestion — carrying raw Fluent syntax in its source and target — was shown alongside the plain match.

flodolo · 2026-07-23T06:30:55Z

Small issue, maybe for a follow-up

It would be great to suggest an accesskey that actually exists in the label. I think pretranslation is already doing that?

Accesskey detection matched the literal lowercase string `accesskey`, so camelCase `accessKey` keys were never recognized. Such an accesskey was translated as an ordinary leaf via TM/MT instead of being derived from its label. The composed Machinery feature only made this visible; pretranslation and the editor's accesskey candidate picker were silently failing on `accessKey`-cased strings all along.

mathjazz · 2026-07-23T11:39:57Z

It would be great to suggest an accesskey that actually exists in the label. I think pretranslation is already doing that?

Accesskey detection was case-sensitive. This was a pre-existing bug beyond this branch — pretranslation and the editor's accesskey picker were silently failing on accessKey-cased strings too. The composed-Machinery feature just made it visible.

If the fix makes sense, please file an issue and I'll close it as part of this PR (working on it separately now will result in merge conflicts).

flodolo · 2026-07-23T11:52:52Z

Accesskey detection was case-sensitive. This was a pre-existing bug beyond this branch — pretranslation and the editor's accesskey picker were silently failing on accessKey-cased strings too. The composed-Machinery feature just made it visible.

Oh, I completely missed it was camel case. I'm not sure that even works in Firefox.

EDIT: there are ~20, so I guess it works.

flodolo · 2026-07-23T13:27:13Z

Besides the 2 bugs I reported (and now fixed), this has been working pretty reliably for me over the last month.

Given the amount of content we're exposing in Firefox these days, I'm hoping that we can push this through the line.

properties) to a source string, which the frontend then re-parsed twice (once to render the rich view, once to distribute across editor fields). The `composed` flag also overloaded MachineryTranslation, so its `original`/`translation` meant "entry source" for composed rows but "plain string" everywhere else. Return the composed (value, properties) as the data-model JSON entities already use (see map_entities), and model composed suggestions with a dedicated ComposedMachineryTranslation type carrying that data model directly. The frontend builds its editor fields straight from it, with no serialize-on-server / re-parse-on-client round trip. - Backend returns value + properties JSON; the no-op guard compares the composed data model against the entity's own. - Composed suggestions live in their own list, deduped by deep-equality on value + properties (merging source badges), rendered above the per-leaf matches with a unified selection index. - New setEditorFromComposed() distributes a pre-built MessageEntry; setEditorFromHelpers() drops its composed-only entry-source param. - MachineryTranslationSource gains a `composed` flag so the Google source shows a plain label (AI refinement stays disabled) instead of routing composed suggestions through GoogleTranslation.

mathjazz · 2026-07-23T15:27:01Z

Besides the 2 bugs I reported (and now fixed), this has been working pretty reliably for me over the last month.

Given the amount of content we're exposing in Firefox these days, I'm hoping that we can push this through the line.

Thanks for testing so extensively!

Sadly I had to refactor quite a bit of code to satisfy @eemeli('s review comments). 😎

The code is now ready for another look and the patch is deployed to https://pontoon.allizom.org/.

eemeli requested changes Jun 22, 2026

View reviewed changes

mathjazz force-pushed the machinery-compose-multi-value-2886 branch from e11f61f to 960112e Compare July 1, 2026 09:42

mathjazz requested a review from eemeli July 1, 2026 09:52

eemeli requested changes Jul 5, 2026

View reviewed changes

mathjazz requested a review from eemeli July 6, 2026 15:27

eemeli requested changes Jul 6, 2026

View reviewed changes

mathjazz added 18 commits July 22, 2026 10:36

Make sure the composed Machinery suggestions are always at the top

fdd8e1c

Fix failing test

f8200da

Add separator between Machinery source titles

4c76fc1

When a suggestion combines multiple sources (e.g. GOOGLE TRANSLATE and TRANSLATION MEMORY), the source titles ran together with no separator.

Rename MT 'provider' to MT 'service' for consistency

c935bc8

Implement proposed walk_entity() changes

ce73fc1

Replace lambdas with direct function calls

0e7ef01

The behaviour should not be gated on the format, but on whether the E…

a88d8c8

…ntity.value and .properties represent a single-pattern or multi-pattern message.

Avoid direct references to the formats in comments

4f3311c

Use match statement for handling services and drop COMPOSED_MT_SERVICES

4153053

Add TODO comment to refactor set_accesskey()

9195b29

Collapse three MT args into one MTEngine enum

a246235

mathjazz added 6 commits July 22, 2026 10:39

Replace the duplicated local COMPOSED_FORMATS set with the shared spe…

7600147

…cialFormats, which is byte-for-byte identical

In hasMultipleFields(), count leaves straight from entity.value and e…

f752aec

…ntity.properties with no parse. Also, document the source-side heuristic.

More accurate code comment

42f7055

Satisfy prettier

dc12571

mathjazz force-pushed the machinery-compose-multi-value-2886 branch from 04ddbd0 to 7f0b5f7 Compare July 22, 2026 10:50

flodolo mentioned this pull request Jul 23, 2026

Pretranslation: ignore case when matching accesskey attribute #4330

Open

mathjazz added 6 commits July 23, 2026 16:07

Assert the composed plural body as one equality

4e86b45

Drop parse_db_string_to_json wrapper

7337ef0

TODO: pretranslate without a fake Entity

c4b5de0

Simplify the composed multi-pattern check

9314ee1

Drop redundant format gate on composed requests

2f38757

mathjazz requested a review from eemeli July 23, 2026 15:29

Uh oh!

Conversation

mathjazz commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

eemeli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mathjazz commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mathjazz commented Jul 1, 2026

Uh oh!

flodolo commented Jul 5, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eemeli Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eemeli left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eemeli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flodolo commented Jul 7, 2026

Uh oh!

flodolo commented Jul 23, 2026

Uh oh!

mathjazz commented Jul 23, 2026

Uh oh!

mathjazz commented Jun 18, 2026 •

edited

Loading

codecov-commenter commented Jun 18, 2026 •

edited

Loading

mathjazz commented Jun 22, 2026 •

edited

Loading

eemeli Jul 6, 2026 •

edited

Loading

eemeli left a comment •

edited

Loading

flodolo commented Jul 23, 2026 •

edited

Loading