Implement iterative precision-based leveraging with tiebreaking#1063
Implement iterative precision-based leveraging with tiebreaking#1063wadimw wants to merge 18 commits into
Conversation
This kind of leveraging should still prefer matches with higher precision, even if they are UNUSED (coming from an older asset that just got removed). Additionally, we'd want it to choose the more recent translation in case of ambiguous matches.
It makes more sense to match name+content from an unused unit, than to downgrade to name-only
Test setup didn't account for setting the USED status on created unit. Combined with the USED filtering bug fixed in previous commit, this resulted in a fall-through to low precision name-only matching. Fixing the matching behaviour caused this string to correctly be considered a high-precision match (due to name AND content equality) and resulted in status preservation (effective post-leveraging status ACCEPTED). Since this test requires effective status NEEDS_TRANSLATION, we need to change second string's content to get a lower-precision match.
Extracted named test helpers that mirror real-life operations (push, translate, getTranslation, etc.) to improve readability and introduce a consistent way to retrieve data between tests.
Add tests covering the expected disambiguation logic for copy-TM leveraging:
1. Match level priority (highest precision wins):
- NAME mode: MD5 > name+content > name-only
- EXACT mode: MD5 > name+content > content-only
2. Match level always beats used status:
- Unused name+content match wins over used name-only match
- Unused name+content match wins over used content-only match
- Unused MD5 match wins over used name+content match
3. Used-over-unused preference within the same match level
4. Translation recency tiebreaker:
- Aligned case: newer TU with newer translation wins
- Diverged case: older TU with newer translation wins, proving
translation timestamp is used rather than TU creation order
5. MD5 matching in NAME mode preserves APPROVED status
…r precision matching
Improved the leveraging logic to be more intuitive and reliable. Unit matching now attempts matching in descending precision order (MD5 > name+content > name-only/content-only). If multiple matches are found at the same precision level, tiebreaking is applied based on USED status and translation recency to select the best available match. Match level ordering per mode: - MD5: MD5 only - NAME: MD5 > name+content > name-only - EXACT: MD5 > name+content > content-only Once any matches are found at a given precision level, iteration ends. Within each level, tiebreaking picks: - used over unused - most recently translated CopyTmLeverager no longer extends AbstractLeverager because this pattern was not scaling well in the iterative approach.
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
da02803 to
67fe126
Compare
|
Showerthought: need tiebreaking by status before recency |
|
To discuss: Do we want to leveraging on a per-locale basis, or stick to whole-unit (all locales together) matches?
EDIT: We've agreed on per-locale for all levels |
Previously, we picked one candidate TMTextUnit for all locales based on aggregated signals (used status, recency across all locales). Now each locale independently picks its best translation using: used over unused, then most recently translated.
Tiebreaking has 3 levels now: used > status > recency
| TextUnitDTO translation = selected.translation; | ||
|
|
||
| TMTextUnitVariant.Status effectiveStatus = | ||
| translationNeeded ? TMTextUnitVariant.Status.TRANSLATION_NEEDED : translation.getStatus(); |
There was a problem hiding this comment.
-
Let's say there is a unit that has matches in 2 different assets by name + content. Now asset 1 has an accepted translation in language 1, and asset 2 has an accepted translation in language 2. Now we get the situation where the matches are considered not unique, so the status will be downgraded to "needs translation", even though there is only 1 variant available for each language that is in the accepted state. Maybe we should be counting uniqueness by locale instead of by text unit? The extreme case is when one of the assets has a match but no translations in it, causing a non-unique match with another asset that does have translations.
-
Instead of downgrading to "needs translation", why don't we downgrade to "needs review"?
A "needs translation" state means we have no translation at all, or there is an old translation that we are really not sure about, or there is an old translation and the source has changed so the translation may need to be updated. In all these scenarios, the translator has to do some work. A "needs review" state means that we have a valid translation, but it may not be correct in this context and therefore it needs to be checked again.
I think it would be reasonable for unique MD5 matches to get downgraded from "accepted" to "needs review" because of the new context, but to go all the way to "needs translation" seems excessive.
- What happens to locales that inherit from a parent? Do they have their own variants? Do they get leveraged?
There was a problem hiding this comment.
Question 1: Per-TU vs per-translation uniqueness check
Let's say there is a unit that has matches in 2 different assets by name + content. Now asset 1 has an accepted translation in language 1, and asset 2 has an accepted translation in language 2. Now we get the situation where the matches are considered not unique, so the status will be downgraded to "needs translation", even though there is only 1 variant available for each language that is in the accepted state. Maybe we should be counting uniqueness by locale instead of by text unit? The extreme case is when one of the assets has a match but no translations in it, causing a non-unique match with another asset that does have translations.
Maybe?
Case 1: high-precision name+content match with full translations
- TU A:
- repo: Source
- asset: one.properties
- id: application.greeting
- content: "Hello"
- translations:
- DE "Hallo" APPROVED
- FR "Bonjour" APPROVED
- TU B:
- repo: Source
- asset: other.properties
- id: application.greeting
- content: "Hello"
- translations:
- DE "Hallo" NEEDS-REVIEW
- FR "Bonjour" NEEDS-REVIEW
- Target TU:
- repo: Target
- asset: target.xliff
- id: application.greeting
- content: "Hello"
Leveraging with source Source, target Target, mode NAME, preserve-status UNIQUE
Current outcome: both DE and FR leveraged, but downgraded to NEEDS-TRANSLATION due to per-TM non-uniqueness.
Expected outcome: both leveraged and APPROVED; both TUs have equal content so the match is "effectively unique", same for the translations.
Case 2: high-precision name+content match with complementary translations
- TU A:
- repo: Source
- asset: one.properties
- id: application.greeting
- content: "Hello"
- translations:
- DE "Hallo" APPROVED
- FR NULL NEW
- TU B:
- repo: Source
- asset: other.properties
- id: application.greeting
- content: "Hello"
- translations:
- DE NULL NEW
- FR "Bonjour" APPROVED
- Target TU:
- repo: Target
- asset: target.xliff
- id: application.greeting
- content: "Hello"
Leveraging with source Source, target Target, mode NAME, preserve-status UNIQUE
Current outcome: both DE and FR leveraged, but downgraded to NEEDS-TRANSLATION due to per-TM non-uniqueness.
Expected outcome: both leveraged and APPROVED; both TUs have equal content so the match is again "effectively unique", translations are actually unique; we're fine with mixing translations from across different assets, since this is a high-precision match so there's little risk of contextual mismatch.
Case 3: low precision name-only match with complementary translations
(this is our primary usecase)
- TU A:
- repo: Source
- asset: one.properties
- id: application.greeting
- content: "Hello {name}"
- translations:
- DE "Hallo {name}" APPROVED
- FR NULL NEW
- TU B:
- repo: Source
- asset: other.properties
- id: application.greeting
- content: "Hello {name}"
- translations:
- DE NULL NEW
- FR "Bonjour {name}" APPROVED
- Target TU:
- repo: Target
- asset: target.xliff
- id: application.greeting
- content: "Hello { name }" (different spacing)
Leveraging with source Source, target Target, mode NAME, preserve-status UNIQUE
Current outcome: both DE and FR leveraged, but downgraded to NEEDS-TRANSLATION due to per-TM non-uniqueness.
Expected outcome: debatable (both leveraged and APPROVED?)
Here we have a low-precision match with complementary translations, but the target TU now has a different spacing than the source TUs. This is a case where we know we want to leverage the translations and preserve their status (hence we use --preserve-status=UNIQUE).
Case 4 (counter-example): low precision name-only match across factually different content
- TU A:
- asset: one.properties
- id: application.greeting
- content: "Hello"
- translations:
- DE "Hallo" APPROVED
- FR NULL NEW
- TU B:
- asset: other.properties
- id: application.greeting
- content: "Hello {name}"
- translations:
- DE NULL NEW
- FR "Bonjour {name}" APPROVED
- Target TU:
- repo: Target
- asset: target.xliff
- id: application.greeting
- content: "Hello { name }" (different spacing)
Leveraging with source Source, target Target, mode NAME, preserve-status UNIQUE
Current outcome: DE "Hallo" and FR "Bonjour {name}" leveraged, both downgraded to NEEDS-TRANSLATION due to per-TM non-uniqueness.
Expected outcome: same as current; these should NOT get upgraded to APPROVED since they are factually different.
So in this case, if we were to move to a simple per-locale uniqueness check, we'd accidentally upgrade status for mismatched translations. Maybe we could layer this somehow, like:
- for md5 matches, just simply NEVER downgrade
- for name+content, maybe also never downgrade? depends on if we consider them good enough
- for low precision name/content only
- check for effectively unique TUs i.e. Case 3 - content A equals content B
- then check for effectively unique translations - though if we already have effectively unique TUs, maybe downgrading isn't even needed anymore
I was actually considering including "effectively unique" checks during implementation, but eventually decided to defer it to a separate PR due to complexity. For now, checking per-TU is the safer approach (more false downgrades, but less risk of false upgrades).
Question 2: Downgrading to "needs review"
Instead of downgrading to "needs translation", why don't we downgrade to "needs review"? A "needs translation" state means we have no translation at all, or there is an old translation that we are really not sure about, or there is an old translation and the source has changed so the translation may need to be updated. In all these scenarios, the translator has to do some work. A "needs review" state means that we have a valid translation, but it may not be correct in this context and therefore it needs to be checked again. I think it would be reasonable for unique MD5 matches to get downgraded from "accepted" to "needs review" because of the new context, but to go all the way to "needs translation" seems excessive.
TBD
Question 3: Leveraging inherited locales
What happens to locales that inherit from a parent? Do they have their own variants? Do they get leveraged?
TBD
There was a problem hiding this comment.
We do want to implement effectively unique checks according to the following:
TU A id: app.greeting content: "Hello" comment: "A greeting" DE: "Hallo"
TU B id: app.greeting content: "Hello" comment: "A greeting" DE: "Hallo"
TUs effectively unique, translations effectively unique => not downgrade
TU A id: app.greeting content: "Hello" comment: "A greeting" DE: "Hallo"
TU B id: app.greeting content: "Hello" comment: "A greeting" FR: "Bonjour"
TUs effectively unique, translations actually unique => not downgrade
TU A id: app.greeting content: "Hello" comment: "A greeting" DE: "Hallo!"
TU B id: app.greeting content: "Hello" comment: "A greeting" DE: "Hallo."
TUs effectively unique, translations differ => DO downgrade
TU A id: app.greeting content: "Hello" comment: "A greeting" DE: "Hallo"
TU B id: app.greeting content: "Hello {name}" comment: "A greeting" FR: "Bonjour {name}"
TUs NOT unique, translations actually unique (case 4) => DO downgrade
There was a problem hiding this comment.
For Q2:
Case id application.status content Free translation in A means Free (not occupied) APPROVED translation in B means Free (no cost) APPROVED => target TU id equal content equal (name+content match) but translations differ => we want to get NEEDS_REVIEW (cause it's a valid translation, but we're not sure if it's valid for the context)
Case:
TU A id: app.greeting content: "Hello" comment: "A greeting"
TU B id: app.greeting content: "Hello {name}" comment: "A greeting with name"
Target TU: app.greeting content: "Hello { name }" comment: "A greeting with name"
Expected: prefer B due to matching comment
counter:
one.properties modal.title content "Popup" comment "title of a modal" FR: "French popup" APPROVED
other.properties modal.title content "Please log in" comment "title of a modal" DE: "Bitte einloggen" APPROVED
if we downgrade to NEEDS_REVIEW due to comment equality rather than NEEDS_TRANSLATION, we get FR "French popup" DE "Bitte einloggen"
TU A id: app.greeting content: "Hello {name}" comment: "A greeting" DE: "Hallo"
TU B id: app.greeting content: "Hello {name}" comment: "A greeting with name" DE: "Hallo."
Target TU: app.greeting content: "Hello {name}" comment: "A greeting with name"
Outcome: matching does not consider TU A because TU B is an MD5 match (TU A would be name+content i.e. lower tier)
EDIT: After discussion, we decided that for now we're NOT implementing "partial downgrade" to NEEDS_REVIEW
There was a problem hiding this comment.
Q3: Fallback to parent is applied during mojito pull - in leveraging it's simply: if a translation for frCA exists (as in a distinct frCA one), then copy it over
There was a problem hiding this comment.
Effectively unique implemented in daa07c5 - ready for rereview @ehoogerbeets
…ades When multiple TUs match at the same level, the match can still be effectively unambiguous if all candidates agree on source content AND translations agree (or are complementary) per locale. In UNIQUE mode, this prevents unnecessary downgrades to TRANSLATION_NEEDED for cases like the same TU existing across multiple assets. PRECISION mode remains strict — only actual uniqueness preserves status (when match precision allows it). Users who want the relaxed behavior opt in with `--preserve-status=UNIQUE`.
Leveraging
Improved the leveraging logic to be more intuitive and reliable. Unit matching now attempts matching in descending precision order (MD5 > name+content > name-only/content-only). If multiple matches are found at the same precision level, tiebreaking is applied on a per-locale basis select the best available translation.
Match level ordering per mode:
Once any matches are found at a given precision level, iteration ends.
Within each level, tiebreaking picks:
Added "effectively unambiguous" check to UNIQUE status preservation mode: if leveraging finds matches across multiple TUs, but all matches agree on source content (across matches for all locales) and translations (per-locale), then they are effectively the same and it's safe to preserve the status of leveraged translation.
Additionally, added missing decision details into the logged leveraging comment which are displayed in UI (to help figure out why something didn't get applied correctly). Examples:
Also added some debug logs for the leveraging logic.
Tests
copyByNamePrefersHigherPrecisionEvenWhenAllUnusedrepresents the situation that bit us (leveraging across assets within the same repository causing name+content matching to be skipped due to USED-only filtering)Followup
CopyTmLeveragerdiverged from theAbstractLeveragermodel, so a followup refactor PR #1064 gets rid of it completely in favour of separate, self-contained leveragers.