fix: skip malformed template samples instead of crashing registration - #70
Conversation
A single malformed sample (slot-only line, unbalanced braces — present in released locale files with translated slot names) raised MalformedTemplate through the training executor and killed the whole registration. Per OVOS-INTENT-4 §6.3/§5.3, each malformed sample is now skipped with a WARN naming the skill, intent, lang, topic and reason; the remaining valid samples are indexed, and the registration is rejected only when no valid sample remains. Applies to the legacy intent/entity paths and the INTENT-4 template/entity handlers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Systems nominal. Checks complete. 🛸I've aggregated the results of the automated checks for this PR below. 🔍 LintI've finished my task! Here's the data you need. 📊 ❌ ruff: issues found — see job log 📋 Repo HealthEnsuring the repo's immune system is strong (aka security checks). 🛡️ ✅ All required files present. Latest Version: ✅ 🚌 Bus CoverageHow deep does the message bus rabbit hole go? 🐇 🔨 Build TestsChecking if all the bolts are tightened. 🔩 ✅ All versions pass
⚖️ License CheckThe license check is now finished. 🏁 ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 🔌 Plugin DetectionI've performed a surgical audit of your plugin's OPM hooks. 🩺 Plugin Info:
OPM Detection:
Entry Point Validation:
⊘ No Issues:
🏷️ Release PreviewThe release banner is being designed! 🎨 Current:
✅ PR title follows conventional commit format. 🚀 Release Channel Compatibility Predicted next version:
🔒 Security (pip-audit)I've performed a digital frisk of this contribution. 👮♂️ ✅ No known vulnerabilities found (49 packages scanned). 📊 CoverageA forensic analysis of your test coverage. 🔍 Per-file coverage (3 files)
Full report: download the An automated high-five for your latest changes! 🖐️ |
…lformedTemplate (#493) test_padacioso_raw_reference_is_rejected expected padacioso's own IntentContainer.add_intent() to reject a sample containing an unresolved <name> reference by raising ovos_spec_tools.expansion.MalformedTemplate, per OVOS-INTENT-1 §3.7 (an inline <name> is an authoring convenience that must never reach an intent engine unresolved). The pinned floor "padacioso>=1.0.0,<3.0.0" resolved to the PyPI-published 1.0.0 release, whose add_intent() calls its own local expand_parentheses() and has no notion of ovos_spec_tools or MalformedTemplate at all — it happily trains on the literal "<thing>" token. The fix for this already landed upstream in padacioso (OpenVoiceOS/padacioso#70, "skip malformed template samples instead of crashing registration") which switched add_intent() to delegate expansion to ovos_spec_tools.expand(), the same function raising MalformedTemplate here. That fix is published on PyPI as prerelease 2.2.1a1, but was never picked up because the ovos-workshop floor only guaranteed 1.0.0. No ovos-workshop code was defective: register_padatious_intent() already correctly resolves inline refs when vocabs are supplied and leaves them raw otherwise (test_inline_reference_resolved_before_engine / test_raw_reference_without_vocabs_is_unresolved both passed before this fix). The dangling case is only rejected once the sample reaches the engine, so the engine itself has to be the one raising — which requires the newer padacioso. Bump the floor to 2.2.1a1, following this repo's established prerelease-floor-pin convention (see the ovos-spec-tools constraint two lines above). Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
What
Registering an intent (or entity) whose sample list contains a single malformed template — a slot-only line or unbalanced braces, both present in released locale files where slot names were translated — raised
MalformedTemplatefromovos_spec_tools.expandinsideadd_intentand crashed the training executor, killing the whole registration (and taking the valid samples down with it).Fix (OVOS-INTENT-4 §6.3 / §5.3)
Guard each sample at the OPM layer (
_valid_samples): a sample that fails expansion is skipped with a WARN carrying the skill_id, intent/entity name, lang, topic and a one-line reason; the remaining valid samples are still indexed. The registration is rejected (with a WARN) only when zero valid samples remain — and a rejected intent is no longer indexed inregistered_intents. The guard covers the legacypadatious:register_intent/register_entitypaths and the INTENT-4ovos.intent.register.template/ovos.entity.registerhandlers. The core library keeps its spec-strict expansion.Tests
test/test_malformed_samples.pyreproduces the exact crash payloads on all four registration paths, asserts the per-sample WARN contents (skill, intent, lang, topic), that valid samples still match end-to-end, and the zero-valid-samples rejection. Full suite green (52 passed) in a fresh uv venv.🤖 Generated with Claude Code