Skip to content

feat: rime tts update log#2127

Open
YiminW wants to merge 1 commit intomainfrom
dev/rime_tts_log
Open

feat: rime tts update log#2127
YiminW wants to merge 1 commit intomainfrom
dev/rime_tts_log

Conversation

@YiminW
Copy link
Copy Markdown
Contributor

@YiminW YiminW commented Apr 3, 2026

No description provided.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 3, 2026

PR Review: feat: rime tts update log

Summary: This PR cleans up logging in the RIME TTS extension — removing a leftover debug print() statement, standardizing log message format, adding LOG_CATEGORY_VENDOR categorization, and adding a new EOS debug log point. The version bump from 0.4.40.4.5 is appropriate.


Positive Changes

  • Removed print() statement (line 104): The print(f"KEYPOINT _process_websocket: {self._build_websocket_url()}") was writing directly to stdout, bypassing the structured logging system entirely. Good removal.
  • Structured log format: The shift from free-form messages like "RIME TTS websocket connected successfully" to key=value style (e.g., "websocket_connected", "websocket_closed: error={e}") is consistent with how other extensions log vendor events and improves machine-parseability.
  • category=LOG_CATEGORY_VENDOR: Adding this to all vendor-related log calls is the correct pattern, matching the approach in elevenlabs_tts2_python, azure_tts_python, and others.
  • New EOS log point: The added send_context_end debug log before sending the eos operation is a useful addition for tracing request lifecycle.

Issues / Observations

1. User content logged at DEBUG level (pre-existing, but worth noting)

# rime_tts.py, _send_text_internal
self.ten_env.log_debug(
    f"send_text_to_tts_server: {message_json}",
    category=LOG_CATEGORY_VENDOR,
)

message_json contains {"text": <user speech content>, "contextId": ...}. This was already logged before this PR (as "KEYPOINT Sending text to RIME TTS: {message_json}"), so it is a pre-existing concern — but since we're touching this log line anyway, it's worth considering whether the full text payload is needed at debug level, or if logging just the contextId and text length would be sufficient to limit user data exposure in logs.

2. Import placement (minor nit)

from ten_ai_base.message import (
    ModuleErrorVendorInfo,
    ModuleVendorException,
)
+from ten_ai_base.const import LOG_CATEGORY_VENDOR   # <-- inserted here
from .config import RimeTTSConfig

Alphabetically within the ten_ai_base group, const would come before message. Not a blocker.


Summary

Clean, focused improvement. The changes are consistent with logging conventions used across the rest of the codebase. No bugs introduced. The one item worth a quick discussion is whether send_text_to_tts_server should log the full text payload or just metadata (length/context ID) to reduce user data in debug logs.

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