feat: add dialog context handling and omit empty text results configuration

[diyuyi-agora]

No description provided.

[github-actions]

PR Review — #2158

Reviewed: feat: add dialog context handling and omit empty text results configuration

Overall this is a well-scoped change with thoughtful defensive coding in the nested path setter. Below are the concerns worth addressing before merge.

Correctness / behavior

1. update_configs mutates config.params before tearing down the old connection (extension.py _run_update_configs).
   The sequence is: mutate → stop_connection() → start_connection(). Because stop_connection() is awaited, other coroutines (e.g. anything triggered by the current websocket loop) can observe the new params while the old WebSocket session is still winding down. Safer order:

   await self.stop_connection()
   _apply_string_field_under_params(params, ...)
   await self.start_connection()
   

   This also makes rollback-on-start_connection-failure cleaner.

2. Dead sent assignment in the not utterances fallback (_on_asr_result):

   sent = await self._send_asr_result_from_text(
       text=result.text, is_final=False, ...
   )
   return

   sent is never read (and is_final=False means it wouldn't drive has_final_result anyway). Drop the sent =.

3. parent_not_dict branch is effectively unreachable for multi-segment paths in _apply_string_field_under_params: once the loop runs, cur is always a dict (or we return early). It fires only for a single-segment path where the caller passed a non-dict params, which the caller already guards against. Not harmful — just dead defense. The docstring's invariant ("params is left unchanged for that traversal") does hold since we only auto-create intermediates when keys are missing, and freshly-created empty dicts cannot then contain non-dict values.

Security

4. No allowlist on field_path. update_configs can rewrite any key under params, including access_key, app_key, api_key, api_url. If this cmd is reachable from untrusted edges, this is a credential-rotation / exfiltration primitive. Consider either (a) an allowlist of modifiable paths (e.g. omit_empty_text_results, request.*) or (b) explicitly document the trust boundary and deny mutation of secret fields.

5. _send_full_client_request logs the request section at info level (volcengine_asr_client.py). The request block can carry corpus.context / hot-words / user-provided prompt text that may include PII. to_json(sensitive_handling=True) was carefully designed to redact — the new log bypasses that path. Suggest log_debug, or selectively redact fields known to carry user content.

6. Exception leak in on_cmd: message=str(e) on the unexpected-exception path can surface stack-level internals to the caller. Consider message="internal_error" for the cmd result and keep the detailed str(e) only in the log line.

Tests

7. Coverage is thin for the new surface area. test_apply_string_field_under_params.py covers only the helper. Worth adding:
   • omit_empty_text_results=True — assert _send_asr_result_from_text returns False and no send_asr_result is dispatched; also check that an empty final result does not flip has_final_result (it previously did — this is a meaningful semantic change).
   • on_cmd update_configs — bad JSON, missing schema_version, non-string value, non-dict payload, unknown field path (after allowlisting, if added). Asserting both StatusCode and the (code, message) properties.
   • That a successful update_configs actually triggers stop_connection + start_connection once (mock them).

Nits

8. Version bump 0.3.20 → 0.4.1 skips 0.4.0. Intentional?

9. PR title mentions "dialog context handling" but the code ships a generic nested-path setter (the docstring only references the prior dialog_ctx clear). If dialog context is the motivating use case, fine — but the title oversells specificity. A more accurate phrasing: feat(bytedance_llm_based_asr): add update_configs cmd and omit_empty_text_results.

10. Per project convention (AGENTS.md), commit subjects are type(scope): lowercase description. The second commit (feat: change update_configs) is vague — consider squashing or clarifying on merge.

11. _update_configs_lock only serializes update_configs against itself, not against the reconnect machinery that also invokes stop_connection/start_connection on error paths. If that's a concern in practice, the two paths need a shared lock.

What looks good

• The helper's fail-closed semantics (no silent overwrite of scalars/lists) and the dedicated test for that case.
• Returning bool from _send_asr_result_from_text and using it to gate has_final_result — correct handling of the new suppression flag.
• schema_version gating on the cmd payload — good forward-compat hygiene.
• Using StatusCode.ERROR + (code, message) properties matches the documented ten_ai_base convention.