Skip to content

refactor(voice): add VoiceProviderConfig base for pluggable providers - #114

Open
xinghaohuang91 wants to merge 5 commits into
refactor/extract-twiml-builderfrom
feat/conversation-relay-provider-config
Open

refactor(voice): add VoiceProviderConfig base for pluggable providers#114
xinghaohuang91 wants to merge 5 commits into
refactor/extract-twiml-builderfrom
feat/conversation-relay-provider-config

Conversation

@xinghaohuang91

Copy link
Copy Markdown
Contributor

Summary

  • Stacked on refactor(voice): extract ConversationRelay TwiML logic into twiml.py #113 — merge that one first.
  • Adds a VoiceProviderConfig base (empty for now) and makes ConversationRelayProviderConfig (née VoiceChannelConfig) inherit it.
  • Widens VoiceChannel.__init__'s config parameter to VoiceProviderConfig | dict[str, Any] | None, so a future provider's config class can be passed without widening the channel's type signature again.
  • ConversationRelayProviderConfig is the only implementation today, so VoiceChannel casts to it after normalizing dict/None input rather than adding an isinstance check — that check becomes necessary (and a create_provider() factory method replaces the cast entirely) once a second provider config exists, in a follow-up PR.
  • No behavior changes — this is pure type-signature groundwork for the provider split.

Test plan

  • make lint / make type-check / make check all pass
  • 876 existing tests pass unmodified
  • tests/ and getting_started/ have zero diff — public API and examples untouched

…onfig type

ConversationRelayProviderConfig now inherits VoiceProviderConfig, and
VoiceChannel.__init__ accepts VoiceProviderConfig | dict | None instead of
the concrete VoiceChannelConfig — so a future provider's config class can
be passed without widening the channel's type signature again.

VoiceChannelConfig is now an alias for ConversationRelayProviderConfig
(flipped from the other direction). Since ConversationRelayProviderConfig
is the only implementation today, VoiceChannel casts to it after
normalizing dict/None input rather than adding an isinstance check —
that check becomes necessary once a second provider config exists.

Also adds empty placeholders for the provider split itself: VoiceProvider
and VoiceProviderConfig.create_provider() (in provider.py), and
ConversationRelayProvider (in conversation_relay.py). VoiceChannel builds
self._provider via config.create_provider() but doesn't use it yet —
VoiceChannel still owns all ConversationRelay logic directly. A follow-up
PR moves that logic onto self._provider.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 26, 2026 18:30
@xinghaohuang91
xinghaohuang91 force-pushed the feat/conversation-relay-provider-config branch from 518dc45 to 2d12015 Compare August 26, 2026 18:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR lays groundwork for a future “pluggable voice providers” architecture by introducing a VoiceProviderConfig base type and a (currently placeholder) provider implementation, while keeping ConversationRelay as the only concrete provider today.

Changes:

  • Added VoiceProvider/VoiceProviderConfig scaffolding and a placeholder ConversationRelayProvider.
  • Renamed the concrete voice config model to ConversationRelayProviderConfig (with VoiceChannelConfig kept as an alias) and added create_provider().
  • Widened VoiceChannel.__init__ to accept VoiceProviderConfig | dict | None and instantiated a provider via config.create_provider().

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/tac/channels/voice/provider.py Introduces base provider + base config (VoiceProviderConfig) with a create_provider() hook.
src/tac/channels/voice/conversation_relay.py Adds placeholder ConversationRelayProvider class for the default provider.
src/tac/channels/voice/config.py Makes ConversationRelayProviderConfig inherit VoiceProviderConfig, implements create_provider(), and aliases VoiceChannelConfig.
src/tac/channels/voice/channel.py Broadens config parameter type, normalizes dict/None input, and creates a provider instance.
src/tac/channels/voice/init.py Re-exports the new provider symbols and ConversationRelay provider/config.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/tac/channels/voice/channel.py
xinghaohuang91 and others added 4 commits August 26, 2026 11:49
create_provider(tac_config) instead of create_provider() — a provider that
talks TwiML (ConversationRelayProvider) needs TACConfig to derive default
URLs (voice_public_domain etc.), and a config object has no other way to
reach it. ConversationRelayProvider's constructor takes tac_config too, in
preparation for building its own TwiMLBuilderConversationRelay in a
follow-up PR.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tion

VoiceProvider.__init__(channel) stores self.channel instead of every
method taking channel as a parameter — a provider only ever serves the
one VoiceChannel that built it (create_provider() is called once, from
VoiceChannel.__init__), so there's no case where threading it through
each call buys anything. create_provider(channel, tac_config) passes it
through; storing the reference before BaseChannel.__init__ finishes is
safe since it's only read later, once the channel is fully constructed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
self.logger = get_logger(self.__class__.__module__), same convention as
BaseChannel, instead of providers borrowing channel.logger. Log lines end
up attributed to the provider that actually emitted them (e.g.
tac.channels.voice.conversation_relay) rather than generically to
tac.channels.voice.channel.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.

3 participants