-
Notifications
You must be signed in to change notification settings - Fork 1
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.
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
ai tutor #57
base: main
Are you sure you want to change the base?
ai tutor #57
Conversation
I've actually been thinking of modifying this bit of code, so that we wouldn't have to rely on cookies so much. Maybe this: if (
thread_id
and (
user
and not user.is_anonymous
and await UserChatSession.objects.filter(user=user, thread_id=thread_id).aexists()
) or (
user.is_anonymous
and await UserChatSession.objects.filter(user=None, thread_id=thread_id).aexists
)
): That might allow people to snoop on other anon users' chat history if they happen to guess an existing thread id, but maybe that's low risk enough to not worry about it. Anyway, it's a problem for another PR. |
PS It's also possible to send the thread_id in the header with curl by adding this parameter: -b "TutorBot_ai_threads_anonymous=<thread_id>,; Path=/; HttpOnly" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works great overall, just the llm assignment and default model setting value for the tutorbot could use some changes.
ai_chatbots/chatbots.py
Outdated
thread_id=thread_id, | ||
model=model or settings.AI_DEFAULT_TUTOR_MODEL, | ||
) | ||
self.llm = ChatOpenAI(model=settings.AI_DEFAULT_TUTOR_MODEL, temperature=settings.AI_DEFAULT_TEMPERATURE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was initially thinking this this line shouldn't be necessary, unless using ChatLiteLLM
in the get_llm
function is breaking something. But I tried it and it is (TypeError: Object of type ChatCompletionMessageToolCall is not JSON serializable
from the tutor library). So this is necessary (or you could override the get_llm
function for this class, either way should work). However, settings.AI_DEFAULT_TUTOR_MODEL
should not default to settings.AI_DEFAULT_MODEL
because that value - "openai/gpt-4o" - is not compatible with the ChatOpenAI
class (it would need to be just "gpt-4o").
Another reason to override the get_llm
function instead for this class is to keep adding whatever additional parameters might be necessary for an AI proxy/gateway like LiteLLM. This works in connecting through the LiteLLM proxy:
def get_llm(self, **kwargs) -> BaseChatModel:
llm = ChatOpenAI(
model=f"{self.proxy_prefix}{self.model}",
**(self.proxy.get_api_kwargs(base_url_key="base_url", api_key_key="openai_api_key")),
**(self.proxy.get_additional_kwargs(self) if self.proxy else {}),
**kwargs,
)
# Set the temperature if it's supported by the model
if self.temperature and self.model not in settings.AI_UNSUPPORTED_TEMP_MODELS:
llm.temperature = self.temperature
# Bind tools to the LLM if any
if self.tools:
llm = llm.bind_tools(self.tools)
return llm
with these env settings:
AI_PROXY_URL=http://litellm:4000
AI_PROXY_AUTH_TOKEN=sk-secret
LITELLM_MASTER_KEY=sk-secret
AI_PROXY_CLASS=LiteLLMProxy
Unfortunately the proxy throws this error (with gpt-4o-mini and gpt-4o and o1):
web-1 | openai.BadRequestError: Error code: 400 - {'error': {'message': "litellm.UnsupportedParamsError: litellm_proxy does not support parameters: {'parallel_tool_calls': False}, for model=gpt-4o. To drop these, set `litellm.drop_params=True` or for proxy:\n\n`litellm_settings:\n drop_params: true`\n\nReceived Model Group=litellm_proxy/gpt-4o\nAvailable Model Group Fallbacks=None", 'type': 'None', 'param': None, 'code': '400'}}
There is a closed issue for anthropic very similar to this: BerriAI/litellm#6456
So the LiteLLM proxy may not be the best choice going forward. I'm hoping to try out the Portkey AI gateway soon.
main/settings.py
Outdated
@@ -587,6 +587,7 @@ def get_all_config_keys(): | |||
"AI_DEFAULT_RECOMMENDATION_MODEL", AI_DEFAULT_MODEL | |||
) | |||
AI_DEFAULT_SYLLABUS_MODEL = get_string("AI_DEFAULT_SYLLABUS_MODEL", AI_DEFAULT_MODEL) | |||
AI_DEFAULT_TUTOR_MODEL= get_string("AI_DEFAULT_TUTOR_MODEL", AI_DEFAULT_MODEL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AI_DEFAULT_MODEL
won't work here as a default with the ChatOpenAI
class because of the openai/
prefix.
*, | ||
extra_state: Optional[TypedDict] = None, | ||
debug: bool = settings.AI_DEBUG, | ||
) -> AsyncGenerator[str, None]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docstring
ai_chatbots/consumers.py
Outdated
|
||
|
||
def create_chatbot(self, serializer: TutorChatRequestSerializer, checkpointer: BaseCheckpointSaver,): | ||
"""Return a SyllabusBot instance""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"""Return a TutorBot instance"""
|
||
|
||
|
||
class TutorBotOutput(models.Model): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docstring
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
conflict with poetry.lock but works great 👍
What are the relevant tickets?
https://github.com/mitodl/hq/issues/6515
Description (What does it do?)
This creates a backend for interacting with the Tutor bot https://github.com/MIT-OL-AI-Tutoring/Open_Learning_AI_Tutor. The verison of the open-tutor-bot that we will import is https://github.com/MIT-OL-AI-Tutoring/Open_Learning_AI_Tutor/pull/7/files. Which is currently a branch off of a branch. I will clean that up before merging this PR.
Currently the tutor uses the demo problems from that repo. Problem codes "A1P1", "A1P2" or "A1P3" work. A future update will integrate this with the contentfiles api.
Currently this does not have a ui. I will build one next
How can this be tested?
Run
Verify you get a response.
From the shell run
Verify that you see "what should i try next?" in the output json
I couldn't figure out how to post as a logged in user from curl so to test the memory i replaced the content of
https://github.com/mitodl/learn-ai/blob/main/ai_chatbots/consumers.py#L76-L83
with
if ( thread_id ):
to be able to assign the thread to a message as an anonymous user. I'm not sure if there is a better way. Do that and runFrom the shell run
again. Verify thread_id is "test" and you see "what should i try next?" in the json
From the shell run
Run
run
again. Verify thread_id is "test" and you see both "what should i try next?" and "what if i took the average" in the json