feat:multilang_match #646
feat:multilang_match #646
Conversation
if enabled attempt to match intents across all configured langs this allows OVOS to match even if lang was incorrectly tagged, as long as the transcription is correct use cases aretext chatbots (eg, matrix hivemind both) or misclassifications of audio transformer plugins
|
Caution Review failedThe pull request is closed. WalkthroughThe changes enhance the Changes
Sequence DiagramsequenceDiagram
participant IntentService
participant MatchFunction
participant Message
IntentService->>IntentService: Prepare language list
loop For each language
IntentService->>MatchFunction: Attempt intent match
alt Match found
IntentService->>IntentService: Process match
else No match
IntentService->>IntentService: Log debug message
end
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #646 +/- ##
==========================================
- Coverage 75.33% 70.98% -4.36%
==========================================
Files 15 15
Lines 3094 1651 -1443
==========================================
- Hits 2331 1172 -1159
+ Misses 763 479 -284
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
ovos_core/intent_services/__init__.py (3)
413-417: Consider optimizing the multilingual matching implementation.The current implementation could be improved in terms of efficiency and robustness:
- Cache the valid languages list to avoid repeated computation
- Allow configuring specific secondary languages instead of using all valid languages
- Validate language support for each matcher function
Consider this implementation:
+ def _get_matching_languages(self, primary_lang): + """Get ordered list of languages to attempt matching against.""" + if not hasattr(self, '_valid_languages'): + self._valid_languages = get_valid_languages() + + # Use configured languages or fall back to all valid languages + secondary_langs = self.config.get("secondary_languages") or \ + [l for l in self._valid_languages if l != primary_lang] + + return [primary_lang] + secondary_langs # In handle_utterance method: - langs = [lang] - if self.config.get("multilingual_matching"): - # if multilingual matching is enabled, attempt to match all user languages if main fails - langs += [l for l in get_valid_languages() if l != lang] + langs = [lang] + if self.config.get("multilingual_matching"): + langs = self._get_matching_languages(lang)
418-437: Enhance observability and error handling in the language matching loop.The language iteration logic could benefit from improved logging and error handling to aid in debugging and monitoring:
- Log attempted languages for better debugging
- Add specific exception handling
- Include performance metrics per language attempt
Consider this implementation:
for l in langs: + LOG.debug(f"Attempting match with language: {l}") + match_start = time.time() match = match_func(utterances, l, message) + match_duration = time.time() - match_start + LOG.debug(f"Match attempt for {l} took {match_duration:.3f}s") if match: LOG.info(f"{pipeline} match ({l}): {match}") if match.skill_id and match.skill_id in sess.blacklisted_skills: LOG.debug( f"ignoring match, skill_id '{match.skill_id}' blacklisted by Session '{sess.session_id}'") continue if isinstance(match, IntentHandlerMatch) and match.match_type in sess.blacklisted_intents: LOG.debug( f"ignoring match, intent '{match.match_type}' blacklisted by Session '{sess.session_id}'") continue try: self._emit_match_message(match, message) break - except: + except Exception as e: + LOG.exception(f"{match_func} returned an invalid match: {str(e)}") LOG.exception(f"{match_func} returned an invalid match") else: - LOG.debug(f"no match from {match_func}") + LOG.debug(f"no match from {match_func} for languages: {langs}") continue
413-437: Add documentation for the multilingual matching feature.The implementation successfully adds multilingual matching capability, but would benefit from documentation explaining:
- Configuration options
- Language selection behavior
- Performance implications
Consider adding a docstring like this:
""" Attempt to match intents across multiple languages when enabled. The matching process will: 1. First attempt to match in the primary language 2. If no match is found and multilingual_matching is enabled, attempt to match in configured secondary languages Configuration: multilingual_matching (bool): Enable/disable multilingual matching secondary_languages (list): Optional list of secondary languages to try. If not specified, all valid languages are used. Example config: { "multilingual_matching": true, "secondary_languages": ["pt-PT", "fr-FR"] } """
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
ovos_core/intent_services/__init__.py (2)
Line range hint
270-281: Ensure docstring clarity for the newlangparameter.
This additional parameter and docstring entry align with the PR's goal of enabling multilingual matching. Consider refining the docstring to convey how this parameter is determined (e.g., primary vs. secondary language) and whether it can differ from what’s inmessage.context.
416-419: Rename ambiguous variablelto improve clarity.
Usinglcan trigger readability warnings. Renaming it avoids confusion.- langs += [l for l in get_valid_languages() if l != lang] + langs += [lang_code for lang_code in get_valid_languages() if lang_code != lang]🧰 Tools
🪛 Ruff (0.8.2)
419-419: Ambiguous variable name:
l(E741)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ovos_core/intent_services/__init__.py(5 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
ovos_core/intent_services/__init__.py
419-419: Ambiguous variable name: l
(E741)
435-435: Do not use bare except
(E722)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: end2end_tests (3.9)
- GitHub Check: mycroft_tests (3.9)
- GitHub Check: unit_tests (3.9)
- GitHub Check: license_tests
🔇 Additional comments (3)
ovos_core/intent_services/__init__.py (3)
300-300: Session language update looks correct.
Updatingsess.langto match the detected language is consistent with the overall multilingual design. No concerns here.
315-315: Storing language in reply data is consistent.
Assigningreply.data["lang"]helps downstream consumers understand the matched language context.
420-434: Review matching order for best results.
The loop prioritizes an immediate match in one language over potentially better matches in others. If desired, you could gather all possible matches across languages and then compare them by confidence. For now, this straightforward approach aligns with the PR objectives.
if enabled attempt to match intents across all configured langs
this allows OVOS to match even if lang was incorrectly tagged, as long as the transcription is correct
use cases aretext chatbots (eg, matrix hivemind both) or misclassifications of audio transformer plugins
example config
{ "lang": "en-US", "secondary_langs": [ "pt-PT", "fr-FR" ], "intents": { "multilingual_matching": true } }Summary by CodeRabbit
New Features
Bug Fixes
Chores