Skip to content

Conversation

@schloerke
Copy link
Collaborator

No description provided.

@schloerke schloerke requested review from Copilot and gadenbuie March 18, 2025 16:10
Copy link
Contributor

Copilot AI left a comment

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 renames the variable "chat_model" to "chat_client" across several files for improved clarity and consistency.

  • Updated variable name in model initialization and usage across multiple language model integrations.
  • Standardized the return value and async function references to use "chat_client".

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
shiny/templates/chat/llms/playground/app.py Renamed variable in model creation and return statement.
shiny/templates/chat/llms/ollama/app.py Updated variable references in initialization and async callback.
shiny/templates/chat/llm-enterprise/aws-bedrock-anthropic/app.py Replaced variable name in initialization and async handler.
shiny/templates/chat/llms/anthropic/app.py Renamed variable and updated its async usage.
shiny/templates/chat/llm-enterprise/azure-openai/app.py Changed variable name in initialization and async callback.
shiny/templates/chat/llms/langchain/app.py Updated variable name in both initialization and async handling.
shiny/templates/chat/llms/google/app.py Renamed variable in initialization and async function.
shiny/templates/chat/llms/openai/app.py Changed variable name in initialization and async callback.

Copy link
Collaborator

@gadenbuie gadenbuie left a comment

Choose a reason for hiding this comment

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

I like it! Thanks 😄

@gadenbuie gadenbuie requested a review from cpsievert March 18, 2025 16:17
@cpsievert
Copy link
Collaborator

I don't love this change for 2 reasons:

  1. The ui.Chat() component could be viewed as a client to the backend chatlas.Chat*(), making it then confusing then which one is a client and which one isn't
  2. The term "Chat Model" was made pretty popular by LangChain, and they use it to effectively mean the same thing as chatlas.Chat*()

https://python.langchain.com/v0.1/docs/modules/model_io/chat/
https://python.langchain.com/v0.1/docs/modules/model_io/chat/streaming/

@gadenbuie
Copy link
Collaborator

  1. The ui.Chat() component could be viewed as a client to the backend chatlas.Chat*(), making it then confusing then which one is a client and which one isn't

I think the risk of confusion about which is the "client" is so much smaller compared to "model". AFAIK, we don't use client to talk about ui.Chat() anywhere, but model is overloaded all over the place:

  1. claude-3-5-sonnet is a model
  2. But if we conflate provider + model then Claude 3.5 Sonnet via ChatAnthropic or ChatBedrock are both chat models that use the same model.
  3. And these are all pydantic models, so the chat model objects all have a bunch of .model_* methods that are an entirely different definition of model.

@cpsievert
Copy link
Collaborator

And these are all pydantic models, so the chat model objects all have a bunch of .model_* methods that are an entirely different definition of model.

I'm not sure I follow. Are you saying chat_model/chat_client inherits from BaseModel?

@gadenbuie
Copy link
Collaborator

I'm not sure I follow. Are you saying chat_model/chat_client inherits from BaseModel?

Oh sorry, no, not specifically the Chat class but you'll encounter pydantic models when interacting with the chat object.

@schloerke schloerke merged commit 9ff95ef into main Mar 19, 2025
54 checks passed
@schloerke schloerke deleted the chat_client branch March 19, 2025 14:03
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.

4 participants