Skip to content

fix(home-assistant): stop a voice satellite asking questions in a loop - #1796

Merged
johnae merged 1 commit into
mainfrom
ha-question-loop
Sep 26, 2026
Merged

johnae merged 1 commit into
mainfrom
ha-question-loop

Conversation

@johnae

@johnae johnae commented Sep 26, 2026 •

Copy link
Copy Markdown
Owner

The bedroom Voice PE went silent: commands still ran, but it never answered.
At 06:42 background speech reached the model, which replied "Okej. Vad vill
du lyssna på?". Home Assistant keeps a conversation open after any reply
ending in "?", so the Voice PE listened again, heard more background speech,
and got another question back. That repeated every 5-10 seconds for about
twenty minutes. The Voice PE queued audio for every reply until its speaker
logged Queue full, URI dropped and stopped accepting audio altogether. It is
still jammed and needs a power cycle.

  • A Home Assistant patch keeps the conversation open for at most two
    questions in a row. That is enough for "Menade du X?", "Nej", "Vad vill du
    lyssna på?", and it cuts a loop off at the third question. Checked
    against the patched module: a single question, that three-step flow, and a
    question after a statement stay open; a third question in a row does not.
  • The prompt (runtime; the docs contain the new text) now says to reply
    "Okej." to anything unclear, to ask only when an answer is needed, and to
    use "Vad vill du lyssna på?" only after a no to "Menade du". That alone
    still let 2 of 4 background phrases get a question back, hence the patch.
  • Wake word sensitivity on both units went from "Slightly sensitive" to
    "Moderately sensitive" (runtime; documented). The kitchen unit caught
    "Okay Nabu" only 20-30% of the time.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XfYTVcQVjCevVeoJofu8FF

@claude

claude Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @johnae's task —— View job


Review

  • Read repo guidelines (DEVELOPMENT.md)
  • Review the changed files
  • Post review feedback

The diagnosis is well documented and the fix is minimal and in the right layer. The diff is well-formed (hunk counts in home-assistant-cap-continued-questions.patch check out: -359,18 +359,32), the overrideAttrs + patches pattern matches profiles/music-assistant.nix:26, and the nix comment explains why rather than what. A few things worth considering.

1. The counter never resets on a genuine user answer (main concern)

profiles/home-assistant-cap-continued-questions.patch:35-41:

for content in reversed(self.content):
    if content.role != "assistant" or not content.content:
        continue

User turns and tool-call turns are continued, not counted and not treated as a boundary, so "two questions in a row" really means "two assistant question-replies with no intervening non-question assistant reply", however many real user turns sit between them. The prompt's own music flow reaches three:

  1. "Menade du Bruce Springsteen?" (1)
  2. user: "nej"
  3. "Okej. Vad vill du lyssna på?" (2)
  4. user: "spela Dolly Style" → Sök musik (tool turns have content=None, so they're skipped)
  5. "Menade du Dolly Style?" (3) → conversation closes, and the user has to say the wake word again to answer "ja"

That is exactly the path the prompt at docs/home-assistant-runtime.md:66 encourages, so the cap will bite in a normal interaction, not just in the loop. A cheap fix that keeps the anti-loop property: break when you hit an assistant turn that carried tool_calls (a tool call means the user's answer actually led somewhere, i.e. it wasn't background speech). Equivalently, count only questions since the last turn that did real work.

2. Small things in the same patch

  • The scan walks the whole conversation on every call. Bounded in practice, but if questions > 2: return False inside the loop makes the intent explicit and exits early.
  • 2 is a bare literal in a return; upstream style would be a module constant (MAX_CONTINUED_QUESTIONS = 2) — also makes the patch easier to re-read a year from now.
  • No provenance header. Neither new patch says which Home Assistant version it was written against or links an upstream issue. A rebase-breaking failure is loud (good), but the person fixing it will want to know the base. Consider a leading comment line in the patch file, or a version in the nix comment.
  • This behaviour is a plausible upstream contribution (unbounded satellite question loops aren't specific to this setup). Filing it upstream is the only way the patch eventually goes away.

3. home-assistant-live-context-single-item-lists.patch (from #1795)

The unwrapping only covers name and area. If the schema also takes floor (and the model copies the list form there too), the same failure returns silently. Worth extending the tuple unless you've confirmed floor isn't in that tool's parameters. Otherwise the transform is well scoped: only length-1 lists, only for those keys, so no argument is silently truncated.

4. Test coverage / verification

Nothing here is covered by a check that CI runs — the verification described in the PR body ("checked against the patched module") is manual and not reproducible from the repo. Two options, in increasing effort: note the exact check in the patch header, or wire upstream's tests/components/conversation/test_chat_log.py into a passthru.tests so a nixpkgs bump that changes continue_conversation fails visibly rather than at 06:42 in the bedroom. Also worth confirming that upstream's existing continue_conversation tests (which assert a question always continues) don't run in any check you do build — if they do, they will now fail.

5. Docs

docs/home-assistant-runtime.md:71-72 still reads "A reply ending in ? keeps the conversation open" with no mention of the new cap, which is the one line a future reader will hit when the third question surprises them. One clause pointing at profiles/home-assistant-cap-continued-questions.patch would close that gap. The new "Voice PE satellites" section is a good addition — the Queue full symptom and the power-cycle-only recovery are exactly the kind of thing that's otherwise lost.

Minor interaction worth keeping in mind: raising wake word sensitivity to "Moderately sensitive" increases false wakes, which is the loop's entry point. The cap bounds each episode at two replies, so this should be fine, but the two changes pull in opposite directions.

Performance / security

No concerns. The added scan is O(conversation length) on a code path that runs once per reply, and nothing here touches credentials or widens exposure — the lock stays unexposed to Assist.


I could not run world lint / world check or build the patched package in this environment (no nix or network access here), so patch application against the pinned nixpkgs home-assistant is unverified — worth a local world build before merge.
· Branch: ha-question-loop

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XfYTVcQVjCevVeoJofu8FF
@claude

claude Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @johnae's task —— View job


Review

  • Read repo guidelines (DEVELOPMENT.md)
  • Review the changed files
  • Post review feedback

The branch is unchanged from my earlier pass (still the single commit ad168de, identical diff), so this is a re-review of the same content. The diagnosis and the layering are still good, and two things improved relative to what I'd have asked for: the nix comment now explains why overrideAttrs rather than overridePythonAttrs, and the added patch follows the repo's existing patch style (profiles/music-assistant-spotify-toptracks.patch also has no provenance header and puts the reasoning in a code comment) — so withdraw my earlier "add a header" point as inconsistent with convention.

One substantive concern remains open.

1. The counter doesn't reset when the user's answer actually did something

profiles/home-assistant-cap-continued-questions.patch:35-37:

for content in reversed(self.content):
    if content.role != "assistant" or not content.content:
        continue

UserContent and assistant turns whose content is None (the tool-call turns) are continued — skipped, not treated as a boundary. So "two questions in a row" means "two assistant question-replies with no intervening assistant reply that didn't end in ?", regardless of how many real user turns and tool calls sit between them.

The prompt's own music flow (docs/home-assistant-runtime.md:60-66) reaches three:

  1. "Menade du Bruce Springsteen?" → 1
  2. user: "nej"
  3. "Okej. Vad vill du lyssna på?" → 2
  4. user: "spela Dolly Style" → Sök musik (tool-call turn, content=None, skipped)
  5. "Menade du Dolly Style?" → 3 → continue_conversation False, mic closes, user must say the wake word again to answer "ja"

So the cap bites in the exact interaction the prompt encourages, not only in the loop. Worse, whether it bites is model-dependent: if gemma happens to emit some text alongside the tool call, that text won't end in ?, the loop breaks, and the count resets — the same conversation behaves differently depending on whether the model was chatty on the tool turn. That non-determinism is the part I'd most want removed.

Making the boundary explicit fixes both, and keeps the anti-loop property (background speech never produces a tool call):

        questions = 0
        for content in reversed(self.content):
            if content.role != "assistant":
                continue
            # A tool call means the previous answer led somewhere, so the
            # questions before it were part of a real exchange, not a loop.
            if getattr(content, "tool_calls", None):
                break
            if not content.content or not content.content.strip().endswith(question_marks):
                break
            questions += 1
            if questions > 2:
                return False
        return True

The if questions > 2: return False inside the loop also bounds the scan at three iterations and states the intent where it happens, instead of walking the whole conversation and deciding at the end.

2. Smaller things

  • return questions <= 2 puts a bare 2 in a return expression; upstream style here would be a module constant (MAX_CONTINUED_QUESTIONS = 2), which also documents the number for whoever rebases this.
  • The initial if not (...) block duplicates what the loop's first iteration checks (the last message is by definition the first thing reversed() yields). Not wrong — the early return is arguably clearer — just worth knowing when the loop is edited later, since the two conditions must stay in sync.
  • The cap applies to every conversation agent, not just satellites: a typed Assist chat also loses continue_conversation after two questions. Harmless (typing doesn't depend on it) but it is wider than the problem.
  • This is a plausible upstream contribution; unbounded satellite question loops aren't specific to this setup, and filing it is the only route to the patch eventually going away.

3. Docs

docs/home-assistant-runtime.md:71-72 still reads "A reply ending in ? keeps the conversation open, so the answer to 'Menade du …?' is heard without the wake word" — with no mention of the cap. That is the one line a future reader lands on when a third question surprises them; one clause pointing at profiles/home-assistant-cap-continued-questions.patch closes the gap. The prompt change at line 56 is good and the new "The Voice PE satellites" section (lines 22-31) is the best part of this PR — the Queue full symptom and "only a power cycle clears it" are exactly what gets lost otherwise.

Worth keeping in mind that raising sensitivity to "Moderately sensitive" increases false wakes, which is the loop's entry point; the cap bounds each episode, so the net is still an improvement, but the two changes pull in opposite directions.

4. Verification

Nothing here is covered by anything CI builds — "checked against the patched module" isn't reproducible from the repo. Two things worth confirming locally before merge:

  • world build on icarus, so patch application against the pinned home-assistant is actually exercised (a context drift here fails the build, which is the good failure mode).
  • Whether upstream's tests/components/conversation/test_chat_log.py runs in any derivation you build. Upstream asserts that a question always continues the conversation, so if those tests execute, they now fail.

Performance / security

No concerns. The scan is O(conversation length) once per reply (three iterations with the early return), and nothing here touches credentials or exposure — the lock stays hidden from Assist.


I could not run world lint, world dead or world build in this environment (no nix or network access), so the nix formatting and patch application are unverified from here.

· Branch: ha-question-loop · View job

@johnae
johnae merged commit bb1244f into main Sep 26, 2026
2 checks passed
@johnae
johnae deleted the ha-question-loop branch September 26, 2026 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant