fix(openai_compat): apply turn-suffix without a user turn + no whitespace before emoji#145
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses two audit findings in the OpenAICompat LLM provider by ensuring (1) the per-turn safety/format suffix is applied even when a request contains no user turn, and (2) streaming output never sends leading whitespace to TTS/firmware before the emoji-prefix enforcement logic runs.
Changes:
- Adjust
_build_messagesto apply the turn-suffix even whenlast_user_idxisNone, including an empty-dialogue fallback. - Adjust
_response_streamto buffer leading whitespace-only deltas and emit the emoji (fallback if needed) before any non-emoji output reaches TTS. - Add a new unit test suite covering suffix placement and streaming emoji/whitespace ordering.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
custom-providers/openai_compat/openai_compat.py |
Fixes turn-suffix placement for no-user-turn dialogues and enforces emoji-first streaming output. |
tests/test_openai_compat.py |
Adds regression tests for suffix placement rules and stream ordering behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
108
to
+123
| last_user_idx = None | ||
| for i, msg in enumerate(dialogue): | ||
| if msg.get("role") == "user": | ||
| last_user_idx = i | ||
| suffix_idx = last_user_idx if last_user_idx is not None else len(dialogue) - 1 | ||
|
|
||
| for i, msg in enumerate(dialogue): | ||
| role = msg.get("role", "user") | ||
| content = msg.get("content", "") | ||
| if i == last_user_idx: | ||
| if i == suffix_idx: | ||
| content = content + _TURN_SUFFIX | ||
| messages.append({"role": role, "content": content}) | ||
|
|
||
| if suffix_idx < 0: # empty dialogue — nothing to append to | ||
| messages.append({"role": "system", "content": _TURN_SUFFIX}) | ||
|
|
…pace before emoji Two audit findings in the OpenAICompat provider (the alternate voice backend). Turn-suffix placement: _build_messages appended _TURN_SUFFIX (emoji-prefix rule + kid-mode filter + length limits) only at the last *user* turn. A request with no user turn — a greeter/system-only injection ending in a system/assistant message — left last_user_idx None, so the suffix was never applied and the model ran completely unconstrained (no emoji, no kid-mode filter). Now fall back to the last message regardless of role, and synthesize a trailing system message when the dialogue is empty. Whitespace-before-emoji: the stream yielded a leading whitespace-only delta to TTS before the emoji-prefix check had non-whitespace to inspect, so the first thing the firmware saw was whitespace, not the emoji it parses into a face animation. Buffer until there's non-whitespace, then emit the (optional) fallback emoji followed by the accumulated leading text. Tests: new tests/test_openai_compat.py (8 cases) — suffix on last user turn, fallback-to-last-message, empty-dialogue synthesis, exactly-once; and stream ordering for whitespace-first, existing-emoji, no-emoji, and all-whitespace. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… cross-test leak test_openai_compat installed a core.utils.textUtils stub (build_turn_suffix -> '[[SUFFIX]]') into sys.modules at collection time and never removed it. pi_voice.py probes `from core.utils.textUtils import build_turn_suffix` and depends on that raising ImportError to reach its real by-path fallback; the leaked stub made the probe succeed, so test_pi_voice's TestSandwichInjection saw '[[SUFFIX]]' instead of the real suffix and failed (3 failures). Snapshot the touched sys.modules keys before stubbing and restore them after exec_module. openai_compat.py binds its names at import time, so the module under test keeps working while the global registry is left clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BrettKinny
force-pushed
the
audit/openai-compat-suffix-emoji
branch
from
64a0f09 to
70f42ce
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
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.
Two audit findings in the OpenAICompat provider (the alternate voice backend; live config uses PiVoiceLLM). Off
main.Turn-suffix placement
_build_messagesappended_TURN_SUFFIX(emoji-prefix rule + kid-mode filter + length limits) only at the last user turn. A request with no user turn — a greeter/system-only injection ending in a system/assistant message — leftlast_user_idxNone, so the suffix was never applied and the model ran completely unconstrained (no emoji, no kid-mode filter). Now: fall back to the last message regardless of role; synthesize a trailing system message when the dialogue is empty.Whitespace before emoji
The stream yielded a leading whitespace-only delta to TTS before the emoji-prefix check had non-whitespace to inspect, so the firmware saw whitespace first, not the emoji it parses into a face animation. Now: buffer until non-whitespace arrives, then emit the (optional) fallback emoji followed by the accumulated leading text.
Tests
New
tests/test_openai_compat.py(8 cases): suffix on last user turn, fallback-to-last-message, empty-dialogue synthesis, exactly-once; stream ordering for whitespace-first / existing-emoji / no-emoji / all-whitespace. Fulltests/suite passes; ruff clean.Deploy
openai_compatis mounted on the live xiaozhi container but inactive (selected_module.LLM: PiVoiceLLM). Not restarting prod just to sync an inactive provider — will fold into the next active-path deploy.🤖 Generated with Claude Code