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.

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

Stop propagating 'id' field from Message object to request payload #10

Merged
merged 3 commits into from
Sep 14, 2024

Conversation

B-Step62
Copy link
Collaborator

@B-Step62 B-Step62 commented Sep 11, 2024

Resolving #6

The new ChatDatabricks implementation propagates the id fields from the input Message objects to the underlying model endpoint. It causes an error because not all endpoints support id field in the input payload e.g. our FMAPIs.

Indeed, the input messages often do not have id fields if it is specified by users. However, when we pass the chat history to the ChatDatabricks class, it contains AIMessage objects from the previous response, which often contain id field.

Indeed, this is regressoin fromthe old community ChatDatabricks, which does not propagate id field. Therefore, this PR simply revert the new class to original behavior.

Testing

  • Unit test
  • Validated with the sample code user provided in the issue ^

Signed-off-by: B-Step62 <[email protected]>
Copy link
Collaborator

@BenWilson2 BenWilson2 left a comment

Choose a reason for hiding this comment

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

LGTM. We should raise this with the serving team to see if an optional id field can be added to those endpoints. application interfaces with these endpoints can probably benefit from the propagation of this metadata.

@B-Step62 B-Step62 merged commit deee25b into langchain-ai:main Sep 14, 2024
12 checks passed
@B-Step62 B-Step62 deleted the fix-id-issue branch September 14, 2024 01:57
tkanhe-karini added a commit to tkanhe-karini/langchain-databricks that referenced this pull request Sep 16, 2024
Stop propagating 'id' field from Message object to request payload (langchain-ai#10)
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