Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
Hello there! Your PR checks are ready for review. ✨I've aggregated the results of the automated checks for this PR below. 📋 Repo HealthEvaluating the repository's overall condition. 📋 ✅ All required files present. Latest Version: ✅ 📡 Channel Compat — stableI've performed a quick audit of the latest commit. 🕵️ 🚧 Channel unresolvable with this checkout — the repo's dependency floors exceed what the channel pins (fleet finding; see the install log). Constraints: https://raw.githubusercontent.com/OpenVoiceOS/OpenVoiceOS/main/constraints-stable.txt ⚖️ License CheckEverything looks good on the legal front. ✅ ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 🔍 LintThe automated checks have finished their work. 🏁 ❌ ruff: issues found — see job log 🔒 Security (pip-audit)A detailed security audit of your contribution. 📝 ✅ No known vulnerabilities found (72 packages scanned). 🔨 Build TestsChecking the structural resonance of the codebase. 🔊 ✅ All versions pass
📡 Channel Compat — testingChecking the alignment of your contribution. 📏 🚧 Channel unresolvable with this checkout — the repo's dependency floors exceed what the channel pins (fleet finding; see the install log). Constraints: https://raw.githubusercontent.com/OpenVoiceOS/OpenVoiceOS/main/constraints-testing.txt Your loyal script, at your command 🫡 |
`__handle_question_query` is bound to the dispatch topic `question:query` (colon-bearing) and answered by `_emit_common_query_response`. OVOS-MSG-1 §5.3 states response() "MUST NOT be used" on a topic that "contains ':'"; `message.response()` derives `T + ".response"` and is undefined for a dispatch topic like this one. The previous shape of this fix switched to `message.reply(...)` but emitted it on COMMON-QUERY-1's spec topic (`ovos.common_query.response`) with a payload keyed `phrase`, when §7.1 of that spec requires the key `utterance`. Emitting the spec topic name with a non-conformant payload is worse than not emitting it, and the spec's request leg (`ovos.common_query.request`, §11.2) was never bound either. Reworked to emit ONLY the legacy `question:query.response` topic via `message.reply(...)`, with the payload unchanged from what ovos-common-query-pipeline-plugin's `ovos_commonqa/opm.py` reads today. Full COMMON-QUERY-1 adoption (spec topic, `utterance` key, request leg) is a later wave done together with that plugin. Fail-before: reverted the source hunk only (tests kept) - 2 of 3 unit tests in test_common_query_response.py failed (spec topic present when it must not be) and the new ovoscope end2end test (test/end2end/test_common_query_reply_e2e.py) failed the same way on a real MiniCroft stack; restoring the fix makes all four pass. Full suite: 719 passed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
33164d8 to
5dbca75
Compare
1 participant
__handle_question_queryhandles the dispatch topicquestion:query, which contains a:. OVOS-MSG-1 §5.3 states thatresponse()"MUST NOT be used" on a topic that "contains ':'" — it derivesT + ".response", and that arithmetic is undefined for a colon-bearing dispatch topic._emit_common_query_responsenow answers only viamessage.reply(...)on the legacyquestion:query.responsetopic, the topic ovos-common-query-pipeline-plugin'sovos_commonqa/opm.pyactually subscribes to today. Its payload is unchanged.The previous shape of this PR also emitted COMMON-QUERY-1's spec topic,
ovos.common_query.response. That topic is not emitted here: the spec's §7.1 requires the payload keyutterance, and this handler producesphrase. Naming the spec topic with a non-conformant payload is worse than not naming it at all, and the spec's request leg (ovos.common_query.request, §11.2) was never bound either. Full COMMON-QUERY-1 adoption — the spec topic, theutterancepayload key, and the request leg — is a later piece of work done together with the common-query plugin, not this fix.Fail-before: with the source hunk reverted (tests kept), 2 of the 3 unit tests in
test/unittests/test_common_query_response.pyfailed because the spec topic was still present, and the new ovoscope end-to-end test (test/end2end/test_common_query_reply_e2e.py, a real MiniCroft stack answering a common query) failed the same way. Restoring the fix makes all four pass. Full suite: 719 passed.