fix: parse recipient and content from request text on Node turns - #30
Merged
NeonDaniel merged 3 commits intoSep 22, 2026
Merged
NeonDaniel merged 3 commits into
NeonDaniel merged 3 commits into
Conversation
CMS_match_message_phrase returns only kind when an sms or email vocab word matches; the extractors run only when no vocab word is present. The Node branch assumed extraction had already happened, so nearly every matched Node request arrived with an empty payload and spoke ErrorDialog. Fall back to the extractors on the request text, as the mobile path does. DraftEmailIntent, a direct Adapt match with no skill_data at all, takes the same fallback via message.data.utterance. Also fix _extract_content_sms returning two values on its early exit when the function's contract is three; every caller unpacks three.
The existing Node tests hand-build skill_data with recipient and content already filled in, which is why the empty-payload path went unnoticed. Add cases where skill_data carries only kind, and where DraftEmailIntent delivers no skill_data at all.
Seen on a Hub: "draft email to emily that says hi" matched DraftEmailIntent, but _extract_content_email only knows "subject", so it took "emily that says hi" as the recipient and found no content. _extract_content_sms already parses "that says" and "saying"; compose the two for Node email requests and share the spoken-address normalization so "sarah at example dot com" still resolves.
NeonDaniel
approved these changes
Sep 22, 2026
Comment on lines
+690
to
+693
| if "dot" in recipient.split(): | ||
| recipient = recipient.replace(" dot ", ".") | ||
| if "at" in recipient.split(): | ||
| recipient = recipient.replace(" at ", "@").lower() |
Member
There was a problem hiding this comment.
Note for later that this should use resource files to support translation, possibly simplifying to regex instead of keyword matching/splitting
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
2 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.
Follow-up to #28. On a real Hub, nearly every Node request that reached this skill spoke
ErrorDialog("Something went wrong and I can't send your message").Cause
CMS_match_message_phrasechecks vocab first. When the utterance containstext,sms, oremailit returns an exact match with onlykindset and never runs_extract_content_sms/_extract_content_email; those run only when no vocab word is present. The Node branch from #28 assumed extraction had already happened, so a vocab match arrived with an empty payload and hit the error path. The mobile path had always handled this by re-running the extractors on the request text; that fallback was not carried into the Node branch.DraftEmailIntentis a direct Adapt match with noskill_dataat all, so it landed in the same error path.The unit tests in #28 missed this because they hand-build
skill_datawith recipient and content already filled in.Change
requestfrom the Common Messaging callback,utterancefrom an Adapt match) before giving up._extract_content_smsreturned two values on its early exit when its contract is three; every caller unpacks three, so a request without "to" raisedValueError. Pre-existing, also affects the mobile path.DraftEmailIntent, but_extract_content_emailonly knowssubject, so it took "emily that says hi" as the recipient and found no content._extract_content_smsalready parses "that says" and "saying"; the Node email path composes the two. The spoken-address normalization ("sarah at example dot com") is lifted into_parse_email_addressso both paths share it;_extract_content_emailbehavior is unchanged.Scope note
The extractors themselves are unchanged. They still require "to" to find a recipient, so "send a text to Sarah that says I'm late" works and "text Sarah that says I'm late" does not. Widening that grammar is a separate change to shared code.
Verification
pytest test/test_skill.py: 17 passedminerva test-resources: 3 passedminerva test-intents --padacioso: 4 passed