feat(manifest): ovos.intent.list honours include_definitions - #2
Conversation
A client that wants to show what can be said asks ovos.intent.list for
one language, then ovos.intent.describe once per intent to get the
sentences; on a hub with ~70 template intents that is ~70 round trips
per language over the satellite session, on a runtime that wedges under
about ten rapid requests.
Let ovos.intent.list take {"include_definitions": true}. Each result row
then also carries "definition", the stored registration payload -- the
same object ovos.intent.describe returns for that registration, so a
template row brings its "samples" (slots in braces) and a keyword row its
keyword sets. The client gets a whole language in one reply. The default
reply is unchanged: without the flag (or with it false) the row is the
plain section 10.1 quintuple plus enabled.
The filters keep working as before: lang is folded through
standardize_lang, and with session_id the rows come from the session's
effective pool, so a satellite's own re-registration wins and carries the
satellite's payload.
Tests (test_intent_manifest.py, fake bus): default reply carries no
definition and include_definitions=False equals it; the attached
definition is byte-for-byte what describe returns; keyword and template
registrations of one intent each carry their own payload; the lang filter
folds "de-de" to the stored "de-DE" and leaves English out; no lang
covers every row; the session effective pool picks the satellite's
registration. 26 passed in the module (19 before); five of the seven new
tests fail against the previous handler. Documented the two manifest
queries in docs/bus-events.md.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe intent list handler now accepts an optional ChangesIntent list definitions
Estimated code review effort: 2 (Simple) | ~15 minutes Merge Risk: ⚪ Minimal · up to Intent listing now optionally returns each registration definition while preserving existing responses by default. The documented behavior and coverage indicate no remaining merge-blocking risk. Sequence Diagram(s)sequenceDiagram
participant Client
participant IntentManifest
participant SessionEffectivePool
Client->>IntentManifest: Request ovos.intent.list with include_definitions
IntentManifest->>SessionEffectivePool: Read matching registrations
SessionEffectivePool-->>IntentManifest: Return registration rows and payloads
IntentManifest-->>Client: Return rows with definition fields
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 5.88% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Why
A client that wants to show what can be said asks
ovos.intent.listfor one language and thenovos.intent.describeonce per intent to get the sentences. On the hub that is ~70 template intents, so ~70 round trips per language over the satellite session, against a runtime that wedges under about ten rapid requests. The platform contract (infra-manifestsdocs/thalovant-platform/contracts.md, "Hub intent inventory") already allows for the shortcut:The Python SDK (0.4.36,
intents.py) and the ported SDKs send the flag and take the fast path whendefinitionis on the row; today no runtime attaches it, so every client falls back to the per-intent describes.What
ovos_core/intent_services/manifest.py,IntentManifest._on_list:{"include_definitions": true}in the message data attachesdefinitionto each result row: the stored registration payload, the same objectovos.intent.describereturns asdefinitions[i].definitionfor that registration. A template row brings itssamples(slots in braces), a keyword row its keyword sets.enabled, byte-for-byte as before.langis folded throughstandardize_lang,skill_idfilters as before, andsession_idstill draws from the session's effective pool (§11.2), so a satellite's own re-registration wins and carries the satellite's payload.Also documented the two manifest queries (
ovos.intent.list/ovos.intent.describeand their responses) indocs/bus-events.md, which did not list them.Tests
test/unittests/test_intent_manifest.py, newTestIntentListDefinitionson the fake bus:definition;include_definitions=Falseequals the default replydefinitionequals whatovos.intent.describereturns for the same registrationlangfilter foldsde-deto the storedde-DEand leaves the English rows outlangcovers every rowsession_id, the satellite's registration wins and its payload is the satellite's; the inherited default keyword registration keeps its ownWhat ran locally
Venv with Python 3.12,
pip install -e .(base dependencies, pre-releases allowed) + pytest:test/unittests/test_intent_manifest.py: 26 passed (19 ondev); 5 of the 7 new tests fail against the previous handler, the two "default unchanged" ones pass on both, as intended.test/unittests/test_intent_service.py+test_intent_service_extended.py(they construct the manifest too): 61 passed.test/unittests: 336 passed, 3 failed. The 3 failures (test_skill_manager.py::TestDeferredLoadingConfigFlag) fail identically on unmodifieddevin this environment (handler-list assertions against the installed ovos-workshop), unrelated to this change._keycontinuation indent.Not run: the OVOS reusable workflows (build tests with the
mycroft,plugins,skills-essential,testextras, ovoscope, license/opm checks) do not execute in this fork (only CodeQL runs here), so there is no CI signal beyond the local runs above.Rollout: the runtime image needs a rebuild and a rollout
Merging this does not change the hub. The runtime image (
thalovant-api/runtime-images/ovos-core-session-blacklists) installs a vendoredovos_core-3.2.1a1-py3-none-any.whlbuilt from the coordinated upstream heads (OpenVoiceOS#808, OpenVoiceOS#844, OpenVoiceOS#846), pinned by sha256 intests/test_runtime_image_patches.py; this fork'sdevis 2.6.3a1 + #1._on_listis identical on this fork, in that wheel, and on upstreamdev(3.2.7a1), so the hunk applies cleanly to whichever source the next wheel is cut from. For it to take effect: rebuild the runtime wheel with this change, rebuild the image, then roll the runtime group (ovos-core-rtg-<group>restart; the hub's agent-bus client backs off up to 60 s after a restart). Until then clients keep taking the per-intent describe path, which still works.Changelog
CHANGELOG.mdis the generated file from the release workflow (update_changelog: true, regenerated from merged PR titles); the conventional title of this PR is the entry, as with #1.🤖 Generated with Claude Code
Summary by CodeRabbit