Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Refactor chat completion creation to a separate method #1197

Closed
wants to merge 5 commits into from
Closed

chore: Refactor chat completion creation to a separate method #1197

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e2c9caf
chore: Refactor chat completion creation to a separate method
mykola-mokhnach-parloa Dec 9, 2024
96c35b5
Add changeset
mykola-mokhnach-parloa Dec 9, 2024
0ed5b63
Fix imports
mykola-mokhnach-parloa Dec 10, 2024
70fc224
Allow defaults override
mykola-mokhnach-parloa Dec 10, 2024
4ee71a2
Apply format
mykola-mokhnach-parloa Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/slimy-eels-drive.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"livekit-plugins-openai": patch
---

Refactor chat completion creation to a separate method
52 changes: 26 additions & 26 deletions livekit-plugins/livekit-plugins-openai/livekit/plugins/openai/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import datetime
import os
from dataclasses import dataclass
from functools import wraps
from typing import Any, Awaitable, Literal, MutableSet, Union

import aiohttp
Expand All @@ -33,6 +34,7 @@
from livekit.agents.types import DEFAULT_API_CONNECT_OPTIONS, APIConnectOptions

import openai
from openai.resources import AsyncCompletions
from openai.types.chat import ChatCompletionChunk, ChatCompletionMessageParam
from openai.types.chat.chat_completion_chunk import Choice

Expand Down Expand Up @@ -626,6 +628,16 @@ def create_azure_client(
tool_choice=tool_choice,
)

@wraps(AsyncCompletions.create)
def create_chat_completions_stream(self, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

Is the goal to be able to edit the parameters of the LLM inside the VoicePipelineAgent?

Copy link
Author

Choose a reason for hiding this comment

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

in general, yes
we'd like to inject custom values to the corresponding openai API in the descendant class, which are either not implemented in the agent for now or that contain customized logic, that would probably not make much sense for this particular (generic) agent implementation

Copy link
Member

Choose a reason for hiding this comment

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

Could the before_llm_cb callback inside PipelineAgent do the work? You can return an LLMStream from there (so you can use chat with any parameter you want)

Copy link
Author

Choose a reason for hiding this comment

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

I assume with the recent changes in main this PR becomes obsolete.
The original intention was to customize parameters passed to the OpenAI completion creation API rather than these that the chat one exposes.

For now I think it would make sense to close this PR since we anyway override _main_task (now called _run) of the llm.LLMStream class

Copy link
Member

@theomonnom theomonnom Dec 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Interesting, I think this PR may solve this too

Copy link
Author

Choose a reason for hiding this comment

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

most likely it won't because

  1. _extra_options cannot override existing arguments
  2. We need the change to be done for the openai, while the above one is done to anthropic

return self._client.chat.completions.create(**{
"model": self._opts.model,
"stream_options": {"include_usage": True},
"stream": True,
"user": self._opts.user or openai.NOT_GIVEN,
**kwargs,
})

def chat(
self,
*,
Expand All @@ -642,45 +654,33 @@ def chat(
parallel_tool_calls = self._opts.parallel_tool_calls
if tool_choice is None:
tool_choice = self._opts.tool_choice
opts: dict[str, Any] = dict()
stream_kwargs: dict[str, Any] = {
"n": n,
}
if fnc_ctx and len(fnc_ctx.ai_functions) > 0:
fncs_desc = []
for fnc in fnc_ctx.ai_functions.values():
fncs_desc.append(build_oai_function_description(fnc, self.capabilities))

opts["tools"] = fncs_desc
stream_kwargs["tools"] = [
build_oai_function_description(fnc, self.capabilities)
for fnc in fnc_ctx.ai_functions.values()
]

if fnc_ctx and parallel_tool_calls is not None:
opts["parallel_tool_calls"] = parallel_tool_calls
stream_kwargs["parallel_tool_calls"] = parallel_tool_calls
if tool_choice is not None:
if isinstance(tool_choice, ToolChoice):
opts["tool_choice"] = {
stream_kwargs["tool_choice"] = {
"type": "function",
"function": {"name": tool_choice.name},
}
else:
opts["tool_choice"] = tool_choice

user = self._opts.user or openai.NOT_GIVEN
stream_kwargs["tool_choice"] = tool_choice
if temperature is None:
temperature = self._opts.temperature

messages = _build_oai_context(chat_ctx, id(self))

cmp = self._client.chat.completions.create(
messages=messages,
model=self._opts.model,
n=n,
temperature=temperature,
stream_options={"include_usage": True},
stream=True,
user=user,
**opts,
)
stream_kwargs["temperature"] = self._opts.temperature
stream_kwargs["messages"] = _build_oai_context(chat_ctx, id(self))
oai_stream = self.create_chat_completions_stream(**stream_kwargs)

return LLMStream(
self,
oai_stream=cmp,
oai_stream=oai_stream,
chat_ctx=chat_ctx,
fnc_ctx=fnc_ctx,
conn_options=conn_options,
Expand Down
Loading