langchain-ai / langchain-google

MIT License
117 stars 147 forks source link

vertexai: refactor: simplify content processing in anthropic formatter #601

Closed jfypk closed 2 days ago

jfypk commented 1 week ago

PR Description

This PR improves content processing in the Anthropic formatter to handle content blocks more elegantly, while also fixing a bug we noticed from switching from Langchain+Anthropic to Langchain+Anthropic-on-Vertex:

{'type': 'invalid_request_error', 'message': 'messages.16: tool_result block(s) provided when previous message does not contain any tool_use blocks'}

We noticed this in this Merge Request: https://gitlab.com/gitlab-org/duo-workflow/duo-workflow-service/-/merge_requests/59. We worked around this by patching ChatAnthropicVertex._format_output, with the way ChatAnthropic._format_output is implemented in the main langchain library.

Key changes:

Preserves content structure for multi-block responses Simplified logic for single text content handling Maintains tool calls functionality with cleaner code No breaking changes to external interfaces All existing tests pass without modification

The changes support better flexibility in content handling while keeping code maintainable.

Current tests already verify core functionality.

Relevant issues

Type

🐛 Bug Fix 🧹 Refactoring

Changes(optional)

Modifies the _format_output method to be more in line with the Anthropic implementation https://github.com/langchain-ai/langchain/blob/e317d457cfe8586e4006b5f41e2c4f1e18a4d58c/libs/partners/anthropic/langchain_anthropic/chat_models.py#L753C5-L778C10

Testing(optional)

Current tests already verify core functionality.

Note(optional)

Reprazent commented 1 week ago

@jfypk Let's update the description a bit more with what this is actually fixing, including some details. In my opinion, this is a bugfix, not just a refactor.

We noticed that when we were switching from using Langchain+Anthropic to Langchain+Anthropic-on-Vertex, that we'd sometimes get failures like this:

{'type': 'invalid_request_error', 'message': 'messages.16: `tool_result` block(s) provided when previous message does not contain any `tool_use` blocks'}

We noticed this in this Merge Request: https://gitlab.com/gitlab-org/duo-workflow/duo-workflow-service/-/merge_requests/59. We worked around this by patching ChatAnthropicVertex._format_output, with the way ChatAnthropic._format_output is implemented in the main langchain library.

This should fix that discrepancy that we think is a bug.

jfypk commented 6 days ago

hi @lkuligin -- Do you know how I can resolve the following errors?

FAILED tests/integration_tests/test_maas.py::test_stream[mistral-nemo@2407] FAILED tests/integration_tests/test_maas.py::test_stream[mistral-large@2407] FAILED tests/integration_tests/test_maas.py::test_astream[mistral-nemo@2407] FAILED tests/integration_tests/test_maas.py::test_astream[mistral-large@2407] FAILED tests/integration_tests/test_maas.py::test_tools[meta/llama-3.2-90b-vision-instruct-maas] FAILED tests/integration_tests/test_vectorstores.py::test_document_storage[datastore_document_storage]

They seem unrelated to my changes and related to the environment.

Thanks!

lkuligin commented 3 days ago

@jfypk please, look at failing unit tests:

  1. I assume we need to add a missing dependency to the test group
  2. if the model hits Google APIs to validate for credentials during initialization, then we need to mock these calls since unit tests runner doesn't have required auth
jfypk commented 2 days ago

hi @lkuligin -- thanks for the help on the failing unit test.

For these integration tests, is there a gcloud environment that I can use to test these integration tests locally? Spinning up my own is proving to take a long time. It looks like the failing tests revolve around the structure of the outputs from the mistral and meta llms.

appreciate any pointers!

lkuligin commented 2 days ago

@jfypk just ignore failing integration tests for maas models, please. Something is happening there, I need to investigate, but it's not related to your PR (and these tests are not part of the release tests).

You can call linters and unit testing locally though as described in the contribution guide: https://github.com/langchain-ai/langchain-google?tab=readme-ov-file#local-development-dependencies

Reprazent commented 2 days ago

@lkuligin Thanks for merging! Is there a cycle at which you release these? So we know when we can update our dependency in https://gitlab.com/gitlab-org/duo-workflow/duo-workflow-service/-/work_items/155.

cc @jfypk

lkuligin commented 1 day ago

let me fix some tests and release in the beginning of the next week, would that work for you? or do you need it urgently?

Reprazent commented 1 day ago

@lkuligin That sounds great! We can wait until next week, thank you.