fix: intent layers #307
fix: intent layers #307
Conversation
intentlayers was using the munged name preffixed with skill_id: and was failing to enable/disable intents
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (
|
![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 (1)
ovos_workshop/decorators/layers.py (1)
214-215: LGTM! Consider adding a safeguard for malformed intent names.The change correctly extracts the intent name by removing the skill_id prefix, which fixes the issue with munged names. This ensures proper enabling of intents.
Consider adding a safeguard for malformed intent names:
- intent_name = intent.split(f"{self.skill_id}:")[-1] + parts = intent.split(f"{self.skill_id}:") + intent_name = parts[-1] if len(parts) > 1 else intent
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ovos_workshop/decorators/layers.py(2 hunks)
🔇 Additional comments (1)
ovos_workshop/decorators/layers.py (1)
227-228: LGTM! Verify impact on existing skills.
The change correctly mirrors the fix in activate_layer, maintaining consistency in how intent names are handled.
Let's verify the impact on existing skills:
✅ Verification successful
Let me gather more information about intent handling in the codebase to verify the impact.
LGTM! The intent name handling changes are safe.
The code inspection confirms that the changes in activate_layer and deactivate_layer methods are consistent with the codebase's intent handling patterns:
- The skill_id:intent_name format is the standard pattern used throughout the codebase
- The enable_intent/disable_intent methods in OVOSSkill expect the intent name without the skill_id prefix
- No other components rely on the munged format of the intent name
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any custom intent layer implementations that might be affected
rg -l "IntentLayers|intent_layers" | \
xargs rg -l "enable_intent|disable_intent" | \
xargs rg -A 5 -B 5 "skill_id:"
Length of output: 7306
Script:
#!/bin/bash
# Search for intent enable/disable patterns to understand usage
rg -A 3 -B 3 "(enable|disable)_intent"
# Also check for any direct skill_id:intent patterns
rg -A 3 -B 3 "skill_id.*:.*intent"
Length of output: 14410
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #307 +/- ##
==========================================
- Coverage 53.41% 51.60% -1.81%
==========================================
Files 37 36 -1
Lines 4362 4195 -167
==========================================
- Hits 2330 2165 -165
+ Misses 2032 2030 -2 ☔ View full report in Codecov by Sentry. |
intentlayers was using the munged name preffixed with skill_id: and was failing to enable/disable intents
Summary by CodeRabbit
Bug Fixes
Refactor