Further consolidate service update settings into a single ServiceUpdateSettingsFrame class

[markbackman]

Hijacked this PR to make the change to simplify the service update settings. There's not a base class for service updates called ServiceUpdateSettingsFrame. There are subclasses for different service types: LLMUpdateSettingsFrame, TTSUpdateSettingsFrame, STTUpdateSettingsFrame.

When these frames are processed, there's an _update_settings handler in ai_services.py that is responsible for facilitating the update. For most values, this is a key lookup. There is special case handling for model and voice needed to ensure keys are mapped correctly.

For each service that has InputParams, those params are part of a _settings object which is updated by the _update_settings handlers.

This change simplifies maintaining settings updates for services. Going forward, the keys are to:

• Unify naming for similar settings across services. In this PR, voice_id and model are used uniformly for services. Previously, the same change was made for language.
• Settings no longer need setter functions. The single _update_settings method will do the trick unless special handling is required. You can see an example of that for Cartesia's speed and emotion params.

Lastly, in this PR, I'm also applying the Language type across all services. Includes must be from the Language enum. Language strings are translated to the correct format for each service before applying them.

[aconchillo]

I'm wondering if this frame should just be:

@dataclass
class TTSUpdateSettingsFrame(ControlFrame):
    settings: Mapping[str, Any]

and then use "gender", etc. where needed. Because we might have tons of other settings and by just looking at this frame we have no idea which setting corresponds to each service.

If we do that, maybe we can just have:

@dataclass
class ServiceUpdateSettingsFrame(ControlFrame):
    settings: Mapping[str, Any]

which is generic for all services. No need for LLMUpdateSettingsFrame or TTSUpdateSettingsFrame.

I know we lose completions and types but another benefit is that these won't have to change at all and people writing their own services (and not sharing them) they can keep using the frame. Actually, the same idea with extra, but we make all extra.

[mattieruth]

if we did that, it could be more like the metrics and we could have defined types for each of the various known services. that said, this would be a pretty major breaking change, yeah?

[mattieruth]

i take that back. the metrics aren't really service specific. i mis-remembered. but there could still be some typing there eventually. maybe start with Any for now?

[mattieruth]

i take it back again, we could still have different types defined for llm vs tts vs stt. I'm not sure where the google specific goes, though. Is there a way google.py could expand? or maybe instead of having all settings by [str, Any], there's a custom_settings that maps to any?

// in frames.py
@dataclass
class UpdateSettingsFrame(ControlFrame):
    settings: SettingsUpdate

// new settingsUpdates.py ? (name tbd)
class SettingsUpdate(BaseModel):
    model: Optional[str] = None
    custom: Optional[Mapping[str, Any]]

class LLMSettingsUpdate(SettingsUpdate):
   ...

[mattieruth]

btw. working it through with mark, i realized that since these frames are created in client code, the typing described above didn't really make sense.

[mattieruth]

i'm not a fan of having such custom service settings in the base class, but given the current way things are setup, it seems like this is the best option. we should consider if there's a better way to handle custom setting updates in the future.

[aconchillo]

I believe it's the same comment I posted. Would my proposal work?

[mattieruth]

yeah, i think we were reviewing at the same time. I like your proposal and commented on how it might be fleshed out a bit more to maintain some typing.

[aconchillo]

I'm wondering if it's worth it to have a self._settings: Dict[str, Any] in the class instead if one field per setting.

Then you could just do:

for key, value in settings.items():
  if key in settings:
      self._settings[key] = value
  else:
      logger.warning(f"Unknown setting for Anthropic LLM service: {key}")

and in the constructor we initialize self._settings = { "key": value, .... }

[aconchillo]

It's just that

            setter = getattr(self, f"set_{key}", None)
            if setter and callable(setter):

feels a bit wrong.

[mattieruth]

how would this function have been used before? i don't see any usage of it in the codebase and i'm trying to think through if removing this custom-looking handling of the settings breaks things

[markbackman]

I misunderstood how this could be used. It wasn't accessible before! So, it should be safe to remove.

[mattieruth]

to expand on above... maybe this needs to override _update_settings() to be something like:

async def _update_settings(self, settings: Dict[str, Any]):
  super()._update_settings(settings)
  self._voice_settings = self._set_voice_settings()
  if self._websocket:
    msg = {"voice_settings": self._voice_settings}	
    await self._websocket.send(json.dumps(msg))

[mattieruth]

apparently this function was not needed to begin with and should be removed and functionality not preserved, so this can be ignored... but not the one above about set_voice

[mattieruth]

same here. this has custom handling that's more than just setting the voice.... updating my theoretical code below to include a potential way to handle it.

[mattieruth]

nevermind. that one can be ignored... i think this needs to be added to custom handle voice changes though:

async def _update_settings(self, settings: Dict[str, Any]):
  prev_voice = self._settings["voice"]
  super()._update_settings(settings)
  if not prev_voice == self._settings["voice"]:
    await self._disconnect()
    await self._connect()

TBH. The fact that this behavior also happens when the model changes, I'm wondering if elevenlabs requires a disconnect/connect to apply ANY changes 🤔

async def _update_settings(self, settings: Dict[str, Any]):
  prev_settings = self._settings
  super()._update_settings(settings)
  if not prev_settings == self._settings:
    await self._disconnect()
    await self._connect()

[mattieruth]

i'll leave further review to aleix. but to me, other than the couple of places that need custom handling I pointed out, this looks like good cleanup!

[aconchillo]

LGTM!

[aconchillo]

LGTM!