🐛 fix(mymemory): hydrate source and target language codes on TM matches - #4710
Open
mauretto78 wants to merge 3 commits into
Open
🐛 fix(mymemory): hydrate source and target language codes on TM matches#4710mauretto78 wants to merge 3 commits into
mauretto78 wants to merge 3 commits into
Conversation
- inject source/target language codes from engine config into decoded MyMemory matches in MyMemory::_decode() so they survive to the response layer - default segment/translation to empty string instead of relying on undefined array keys when the API omits them - hydrate source/target and tm_properties fields on GetMemoryResponse::buildMyMemoryMatch() so they reach the Matches object - add unit tests covering missing segment/translation, missing tm_properties, and source/target hydration
🧪 Test-Guard Report✅ PASS — All changed source files have adequate test coverage. Coverage Analysis: ✅ PASSChanged lines: 100.0% covered (threshold: 80%) 📋 2 files: 2 ✅ pass
Result: ✅ PASS |
🧪 Test-Guard ReportCoverage Analysis: ❌ FAILChanged lines: 100.0% covered (threshold: 80%) 📋 5 files: 3 ❌ fail, 2 ✅ pass
Test File Matching: ❌ FAILFile matching: 1 pass, 4 fail 📋 5 files: 4 ❌ fail, 1 ✅ pass
Per-File Evaluation:
|
| File | Verdict | Reason |
|---|---|---|
lib/Utils/Engines/MyMemory.php |
✅ pass | shortcut → coverage 100% ≥ 80% |
lib/Utils/Engines/Results/MyMemory/GetMemoryResponse.php |
✅ pass | shortcut → coverage 100% ≥ 80% |
public/js/components/segments/SegmentFooterMultiMatches.js |
No coverage data; new branches for source/target penalty display untested. | |
public/js/components/segments/SegmentFooterTabMatches.js |
No coverage data; added penalty tooltip and conditional rendering not verified by tests. | |
public/js/components/segments/TabConcordanceResults.js |
No coverage data; added conditional styling and refs without test verification. |
Result:
Why this FAIL?
- Coverage Analysis: The JavaScript files under public/js/components/segments (SegmentFooterMultiMatches.js, SegmentFooterTabMatches.js, TabConcordanceResults.js) are missing from the coverage report, indicating a coverage configuration issue that prevents coverage data collection → update coverage instrumentation/configuration to include these files.
- Test File Matching: No matching test files were found for lib/Utils/Engines/MyMemory.php and the three JavaScript segment components, meaning these source files lack associated tests → consider adding or linking appropriate tests for these files.
- Per-File Evaluation: The three JavaScript segment files have new or changed UI logic (penalty display, tooltips, conditional rendering, styling, refs) that are untested due to missing coverage data → add tests to verify these UI behaviors.
- Test File Matching: The PHP file lib/Utils/Engines/Results/MyMemory/GetMemoryResponse.php has a matching test file modified in this PR and full coverage → no action needed.
To resolve: configure coverage to include the JavaScript segment files and add or link tests that cover their new or changed UI logic.
Ostico
requested changes
Jul 24, 2026
| $decoded['matches'][$pos]['segment'] = $match['segment'] ?? ''; | ||
| $decoded['matches'][$pos]['translation'] = $match['translation'] ?? ''; | ||
| $decoded['matches'][$pos]['target_note'] = $match['target_note'] ?? ''; | ||
| $decoded['matches'][$pos]['source'] = $this->_config['source'] ?? $match['source'] ?? ''; |
Contributor
There was a problem hiding this comment.
Here, we are using the match language returned by MyMemory as a fallback—prioritizing it over the language defined in the class configuration, which might eventually be populated with Matecat's source/target values. Currently, this fallback has no effect because $this->_config['source'/'target'] are always null, but if we start using those values later, the feature will break. The two parameters need to be swapped.
Contributor
Author
There was a problem hiding this comment.
Good catch, fixed — swapped the precedence in MyMemory::_decode() so $match['source']/$match['target'] now win over $this->_config, with config only used as a fallback when the match omits them. Added two regression tests in DecodeMyMemoryTest covering both directions (match wins when present, config used when absent). Pushed as 539447f.
Ostico noted the fallback precedence was backwards: _decode() prioritized $this->_config['source'/'target'] over the value MyMemory returns on the match itself. That currently has no visible effect since _config is never populated with those keys, but would silently break real match language codes the moment it is. Swap the precedence so the match's own value wins, falling back to config only when the match omits it, and add regression tests for both directions.
Contributor
Author
|
@Ostico Addressed your review comment — fixed the precedence swap in |
🧪 Test-Guard ReportCoverage Analysis: ❌ FAILChanged lines: 100.0% covered (threshold: 80%) 📋 5 files: 3 ❌ fail, 2 ✅ pass
Test File Matching: ❌ FAILFile matching: 1 pass, 4 fail 📋 5 files: 4 ❌ fail, 1 ✅ pass
Per-File Evaluation:
|
| File | Verdict | Reason |
|---|---|---|
lib/Utils/Engines/MyMemory.php |
✅ pass | shortcut → coverage 100% ≥ 80% |
lib/Utils/Engines/Results/MyMemory/GetMemoryResponse.php |
✅ pass | shortcut → coverage 100% ≥ 80% |
public/js/components/segments/SegmentFooterMultiMatches.js |
No coverage data; changes add UI elements and conditional classes without test evidence. | |
public/js/components/segments/SegmentFooterTabMatches.js |
No coverage data; adds conditional UI elements and tooltips, no test evidence. | |
public/js/components/segments/TabConcordanceResults.js |
No coverage data; adds UI elements with conditional classes, no test evidence. |
Result:
Why this FAIL?
- Coverage Analysis: The JavaScript files under public/js/components/segments (SegmentFooterMultiMatches.js, SegmentFooterTabMatches.js, TabConcordanceResults.js) are missing from the coverage report entirely → likely a coverage configuration issue (source filter or instrumentation) that prevents coverage data collection for these files.
- Test File Matching: No matching test files found for the changed JavaScript segment files and for lib/Utils/Engines/MyMemory.php → tests are missing or not named to match these source files, so test presence is unconfirmed.
- Per-File Evaluation: AI flagged the three segment JS files for adding UI elements and conditional logic without any test evidence → indicates missing tests for new UI behavior.
- Per-File Evaluation: lib/Utils/Engines/MyMemory.php and its related GetMemoryResponse.php have full coverage and matching tests → no action needed for these files.
To resolve: Add or configure test coverage and matching tests for the changed JavaScript segment files and add tests for lib/Utils/Engines/MyMemory.php, and fix coverage instrumentation to include these JS files.
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
3 participants
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
MyMemory TM matches were losing their source/target language codes, and
segment/translationcould end upnullwhen the API response omitted those keys. This fixesMyMemory::_decode()andGetMemoryResponse::buildMyMemoryMatch()to hydratesource/targetfrom the engine config and to always defaultsegment/translationto an emptystring.
Type
feat— new user-facing featurefix— bug fixrefactor— restructure without behavior changechore— build, deps, config, docsperf— performance improvementtest— test coverageChanges
lib/Utils/Engines/MyMemory.phpsource/targetfrom$this->_configinto decoded matches; defaultsegment/translationto''when absentlib/Utils/Engines/Results/MyMemory/GetMemoryResponse.phpsource/target(and null-safetm_properties) ontoMatchesinbuildMyMemoryMatch()tests/unit/Core/Engines/Results/MyMemory/GetMemoryResponseTest.phpsegment/translation, missingtm_properties, andsource/targethydrationTesting
vendor/bin/phpunit --exclude-group=ExternalServices --no-coveragepasses./vendor/bin/phpstanpasses (0 errors, with baseline)Ran the full suite in Docker: 9222 tests, 30003 assertions. 4 pre-existing failures in
CommentControllerTest(broker-unavailable scenarios) are environment-dependent andunrelated to this change — they also fail on
developin this environment since theActiveMQ broker container is reachable rather than down.
phpstanrun clean (0 errors) on all three changed files.AI Disclosure
Claude Code (claude-sonnet-5)
Notes
None.