AUDIO-1: audio output service specification#38
Open
JarbasAl wants to merge 1 commit into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughAdds the OVOS-AUDIO-1 Audio Output Service specification ( ChangesOVOS-AUDIO-1 Audio Output Service Specification
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
JarbasAl
force-pushed
the
spec/audio
branch
4 times, most recently
from
607cf56 to
4a35082
Compare
JarbasAl
changed the title
3 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@audio.md`:
- Line 143: The spec mixes "synthesise/synthesizes/synthesised" and
"synthesize/synthesizes/synthesized"; pick one spelling (e.g., American
"synthesize/synthesizes/synthesized" or British
"synthesise/synthesises/synthesised") and normalize all occurrences accordingly
(including the instance at line with "The audio output service synthesises the
utterance text into audio." and the occurrences around 385-386), updating
headings, body text, and examples so the chosen variant is used consistently
throughout the document.
- Around line 115-129: Add a language identifier to the fenced flow-diagram
block that begins with "ovos.utterance.speak" so markdown tooling treats it as
plain text: change the opening triple-backtick fence to use "text" (i.e.,
```text) for the block that contains the lines "[dialog transformers] ←
OVOS-TRANSFORM-1 §3.5", "[tts transformers] ← OVOS-TRANSFORM-1 §3.6", and
"scheduled playback queue → audio output" so the diagram is correctly typed by
renderers.
In `@ovos-pipeline-1.md`:
- Around line 1150-1158: The spec has a normative conflict: the `listen: true`
MUST on messages emitted as `ovos.utterance.speak` in a `get_response` flow
(OVOS-CONVERSE-1 §5) conflicts with the later statement that handlers have “no
normative obligation” (currently §11); resolve by choosing one approach and
updating the doc accordingly — either move the `listen` requirement out of
handler text into the orchestrator/framework contract (mentioning
`get_response`, `ovos.utterance.speak`, and `listen` so handlers remain
implementation-neutral), or keep the handler-level MUST and update §11 to add
explicit handler conformance obligations requiring handlers to set `listen:
true` when emitting `ovos.utterance.speak` in `get_response` flows; apply the
same change consistently where the spec references handler obligations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 32f3ebb8-0636-4bc4-be10-56cf26a76b09
📒 Files selected for processing (2)
audio.mdovos-pipeline-1.md
Comment on lines
+115
to
+129
| ``` | ||
| ovos.utterance.speak | ||
| │ | ||
| ▼ | ||
| [dialog transformers] ← OVOS-TRANSFORM-1 §3.5 | ||
| │ | ||
| ▼ | ||
| TTS synthesis (text → audio data) | ||
| │ | ||
| ▼ | ||
| [tts transformers] ← OVOS-TRANSFORM-1 §3.6 | ||
| │ | ||
| ▼ | ||
| scheduled playback queue → audio output | ||
| ``` |
There was a problem hiding this comment.
Add a language identifier to the fenced block.
The flow diagram fence is untyped; please mark it as text for markdown tooling compatibility.
Proposed edit
-```
+```text
ovos.utterance.speak
│
▼
[dialog transformers] ← OVOS-TRANSFORM-1 §3.5
@@
scheduled playback queue → audio output</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 115-115: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@audio.md` around lines 115 - 129, Add a language identifier to the fenced
flow-diagram block that begins with "ovos.utterance.speak" so markdown tooling
treats it as plain text: change the opening triple-backtick fence to use "text"
(i.e., ```text) for the block that contains the lines "[dialog transformers] ←
OVOS-TRANSFORM-1 §3.5", "[tts transformers] ← OVOS-TRANSFORM-1 §3.6", and
"scheduled playback queue → audio output" so the diagram is correctly typed by
renderers.
|
Actionable comments posted: 0 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
audio-out.md (1)
137-398:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse one spelling family for “synthesise/synthesize” across the file.
Line 137 uses
synthesiseswhile Line 398 usessynthesized. Please normalize to one variant throughout the spec.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@audio-out.md` around lines 137 - 398, The document inconsistently uses the British "synthesise/synthesises" and American "synthesize/synthesized" spellings; pick one spelling family and normalize every occurrence (e.g., replace "synthesises", "synthesise" and "synthesised" or alternatively "synthesizes", "synthesize" and "synthesized") across the file so all mentions (including headings like "TTS transformer stage", sentence bodies such as the first paragraph and the section titles/notes) use the chosen variant consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@audio-out.md`:
- Around line 238-255: The subsection numbering jumps from "### 4.3 Synthesised
audio delivery — `ovos.audio.speech`" to "### 4.5 Listen flag", leaving out 4.4;
update the heading "### 4.5 Listen flag" to "### 4.4 Listen flag" (or renumber
subsequent headings accordingly) so section references are consistent, and
verify any cross-references in the document that mention 4.5/4.4 are adjusted to
the new number.
---
Outside diff comments:
In `@audio-out.md`:
- Around line 137-398: The document inconsistently uses the British
"synthesise/synthesises" and American "synthesize/synthesized" spellings; pick
one spelling family and normalize every occurrence (e.g., replace "synthesises",
"synthesise" and "synthesised" or alternatively "synthesizes", "synthesize" and
"synthesized") across the file so all mentions (including headings like "TTS
transformer stage", sentence bodies such as the first paragraph and the section
titles/notes) use the chosen variant consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f3c644e8-83c2-48a0-8ba5-435824deff4a
📒 Files selected for processing (4)
appendix/divergences.mdappendix/rationale.mdaudio-out.mdovos-pipeline-1.md
✅ Files skipped from review due to trivial changes (1)
- appendix/divergences.md
🚧 Files skipped from review as they are similar to previous changes (1)
- ovos-pipeline-1.md
Member
Author
|
Merge-ready (MERGEABLE, dev merged in). Template conformance: header present (OVOS-AUDIO-1 v1 Draft), RFC-2119 boilerplate present. Fixed: OVOS-AUDIO-1 was absent from README spec table and CHANGELOG — both added. Spec filename |
JarbasAl
added a commit
that referenced
this pull request
Jun 22, 2026
…isten field Consolidates the PIPELINE-1 companion edits previously bundled into the union-slots (#56), FALLBACK-1 (#39), COMMON-QUERY-1 (#40) and AUDIO-1 (#38) feature PRs into a single one-file change to ovos-pipeline-1.md. - §6.1/§6.2 — orchestrator backstop for required_slots (INTENT-3 §5.3): the orchestrator treats a Match as declined if any required slot is absent. Second line of defense behind engine-side enforcement. - §7.3 — reserve intent_names "fallback" (FALLBACK-1 §6.3) and "common_query" (COMMON-QUERY-1 §3). COMMON-QUERY-1 asserted the reservation but never registered the row; this closes that gap. - §9.6 — add the OPTIONAL listen field to ovos.utterance.speak; the output-side behaviour is owned by AUDIO-1. All additions are backwards-compatible. PIPELINE-1 is already V2 (its namespaced topics replace the pre-spec names); these refinements do not change the class, so the Version stays 2. Adds the missing PIPELINE-1 CHANGELOG section. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
JarbasAl
changed the title
JarbasAl
force-pushed
the
spec/audio
branch
2 times, most recently
from
30743e1 to
bda203b
Compare
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
1 participant
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.
Companion issue: #49
Summary
Defines the audio output service — the pipeline's output-side counterpart that consumes
ovos.utterance.speakand renders natural-language responses as audio.What the spec covers
ovos.utterance.speak) and sound effectsovos.audio.queuefor scheduled playback in queue orderovos.audio.play_sound(plays without queuing)ovos.audio.output.started/ovos.audio.output.ended(session identity fromcontext.session.session_id)ovos.audio.is_speaking(session-scoped via context, not data)ovos.audio.stopand universalovos.stop; MAY scope response to sessionovos.mic.listenemitted after playback ends whenlisten: trueon the speak messageBus surface
ovos.utterance.speakovos.audio.queueovos.audio.play_soundovos.audio.stopovos.audio.is_speakingovos.audio.output.startedovos.audio.output.endedovos.mic.listenSummary by CodeRabbit