langchain
anthropic[minor]: Adds streaming tool call support for Anthropic
#22687
Merged
Overview
Commits
19
Changes
View On GitHub

anthropic[minor]: Adds streaming tool call support for Anthropic #22687

efriis merged 19 commits into master from jacob/anthropic_streaming_tools
jacoblee93
jacoblee93350 days ago (edited 350 days ago)

Preserves string content chunks for non tool call requests for convenience.

One thing - Anthropic events look like this:

RawContentBlockStartEvent(content_block=TextBlock(text='', type='text'), index=0, type='content_block_start')
RawContentBlockDeltaEvent(delta=TextDelta(text='<thinking>\nThe', type='text_delta'), index=0, type='content_block_delta')
RawContentBlockDeltaEvent(delta=TextDelta(text=' provide', type='text_delta'), index=0, type='content_block_delta')
...
RawContentBlockStartEvent(content_block=ToolUseBlock(id='toolu_01GJ6x2ddcMG3psDNNe4eDqb', input={}, name='get_weather', type='tool_use'), index=1, type='content_block_start')
RawContentBlockDeltaEvent(delta=InputJsonDelta(partial_json='', type='input_json_delta'), index=1, type='content_block_delta')

Note that delta has a type field. With this implementation, I'm dropping it because merge_list behavior will concatenate strings.

We currently have index as a special field when merging lists, would it be worth adding type too?

If so, what do we set as a context block chunk? text vs. text_delta/tool_use vs input_json_delta?

CC @ccurme @efriis @baskaryan

jacoblee93 Adds streaming tool call support for Anthropic
0a695968
efriis efriis added partner
efriis efriis assigned efriis efriis 350 days ago
vercel
vercel350 days ago (edited 343 days ago)

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Jun 14, 2024 6:05am
dosubot dosubot added size:L
dosubot dosubot added 🔌: anthropic
dosubot dosubot added 🤖:improvement
efriis
efriis350 days ago

Game to add type to that special list

efriis
efriis commented on 2024-06-07
Conversation is marked as resolved
Show resolved
libs/partners/anthropic/tests/integration_tests/test_chat_models.py
148148
149async def test_async_tool_use() -> None:
150 llm = ChatAnthropic( # type: ignore[call-arg]
151
 model="claude-3-sonnet-20240229",
efriis350 days ago
Suggested change
model="claude-3-sonnet-20240229",
model=MODEL_NAME,
Conversation is marked as resolved
Show resolved
libs/partners/anthropic/tests/integration_tests/test_chat_models.py
314363def test_tool_use() -> None:
315364 llm = ChatAnthropic( # type: ignore[call-arg]
316 model="claude-3-opus-20240229",
365
 model="claude-3-sonnet-20240229",
efriis350 days ago
Suggested change
model="claude-3-sonnet-20240229",
model=MODEL_NAME,
Conversation is marked as resolved
Show resolved
libs/partners/anthropic/tests/integration_tests/test_chat_models.py
340389
341390 # Test streaming
342391 first = True
392
 chunks = [] # type: ignore
efriis350 days ago

did you mean to add something else here operating on chunks? Could we just assert not first instead of chunk length otherwise or remove change?

jacoblee93350 days ago

It's checking to make sure we get multiple message chunks via the assert below.

Conversation is marked as resolved
Show resolved
libs/partners/anthropic/langchain_anthropic/chat_models.py
1094 ):
1095 if coerce_content_to_string:
1096 warnings.warn("Received unexpected tool content block.")
1097
 # TODO: Pass type through here
efriis350 days ago

should we make all the keys line up here (and store a content_block field), or better to re-form?

jacoblee93350 days ago

Purely because of type string fields - right now they would concatenate repeatedly. Could change in core but am wary of magic params.

ccurme347 days ago

I think we will need the type field to be able to send these messages back to Anthropic (we also raise ValueError without them).

I'm also on board to add type as another magic param in core. The risk would be if providers stream out values from the "type" key.

Not 100% on this but another idea is to keep track of a mapping from index to type within ChatAnthropic and pop out the type key from all chunks after the first one per value of index.

Conversation is marked as resolved
Show resolved
libs/partners/anthropic/langchain_anthropic/chat_models.py
11141068 event: anthropic.types.RawMessageStreamEvent,
11151069 *,
11161070 stream_usage: bool = True,
1071
 coerce_content_to_string: bool = False,
efriis350 days ago

is this to avoid unnecessarily turning everything into a list of dicts instead of a str? IMO might be "less is more" and change everything to list of dict

jacoblee93350 days ago (edited 350 days ago)

Wanted to limit the amount of adjustment for users for now - and wasn't sure how StrOutputParser would handle it. I think it's nicer to keep since if you're calling tools you expect some more complex output.

ccurme
ccurme commented on 2024-06-10
Conversation is marked as resolved
Show resolved
libs/partners/anthropic/langchain_anthropic/chat_models.py
696671 msg = _make_message_chunk_from_anthropic_event(
697 event, stream_usage=stream_usage
672 event,
673 stream_usage=stream_usage,
674
 coerce_content_to_string=(not _tools_in_params(params)),
ccurme347 days ago

micro-nit: compute _tools_in_params(params) outside streaming loop?

Conversation is marked as resolved
Show resolved
libs/partners/anthropic/langchain_anthropic/chat_models.py
1105 "index": event.index,
1106 "id": event.content_block.id,
1107 "name": event.content_block.name,
1108
 "args": "",
ccurme347 days ago

do we need an empty tool call chunk?

jacoblee93347 days ago

Great point on passing back to Anthropic, will update and add a test to ensure it works as expected.

jacoblee93 Merge branch 'master' of https://github.com/langchain-ai/langchain in…
db05e0c5
jacoblee93 Update chunking
7dab7d62
jacoblee93 Treat type as a special field when merging lists
e5479e16
jacoblee93
jacoblee93347 days ago

Blocked on #22750 per above discussions on treating type as a special field.

jacoblee93 Lint
929ed2dc
jacoblee93 jacoblee93 marked this pull request as draft 347 days ago
jacoblee93 Merge branch 'master' of https://github.com/langchain-ai/langchain in…
7571959b
jacoblee93 Remove logged warning
e78aa84a
jacoblee93 Merge branch 'jacob/merge_content_types' of https://github.com/langch…
37f287c4
jacoblee93 Update _merge.py
2bb5ba09
jacoblee93 Merge branch 'jacob/merge_content_types' of https://github.com/langch…
fd52d3cf
jacoblee93 Add roundtrip test
c807a413
jacoblee93 Make empty string concatenation a no-op
ffb03545
jacoblee93 Merge branch 'jacob/merge_content_types' of https://github.com/langch…
2fa851cc
jacoblee93 Merge
3b8bcac7
jacoblee93 Combine var
5ad981d5
jacoblee93 jacoblee93 marked this pull request as ready for review 344 days ago
jacoblee93 Fix lint
3ea9e821
jacoblee93 Lint
53d9a5df
efriis
efriis commented on 2024-06-14
Conversation is marked as resolved
Show resolved
libs/partners/anthropic/tests/integration_tests/test_chat_models.py
188 assert len(chunks) > 1
189 assert isinstance(gathered, AIMessageChunk)
190 assert isinstance(gathered.tool_call_chunks, list)
191
 assert len(gathered.tool_call_chunks) == 1
efriis344 days ago

why is this only 1 chunk? Couldn't it be multiple that sum together into a single tool call?

jacoblee93343 days ago (edited 343 days ago)

The chunks here combine into one tool call as they are aggregated

Conversation is marked as resolved
Show resolved
libs/partners/anthropic/tests/integration_tests/test_chat_models.py
428 )
429 chunks = [] # type: ignore
430 first = True
431
 for chunk in stream:
432
 chunks = chunks + [chunk]
433
 if first:
434
 gathered = chunk
435
 first = False
436
 else:
437
 gathered = gathered + chunk # type: ignore
efriis344 days ago

might be good to assert some qualities of the chunks, like content is always a string (albeit sometimes empty) etc

jacoblee93343 days ago

We get a few different things

Conversation is marked as resolved
Show resolved
libs/partners/anthropic/langchain_anthropic/chat_models.py
11251073 input_tokens = event.message.usage.input_tokens
11261074 message_chunk = AIMessageChunk(
1127 content="",
1075
 content="" if coerce_content_to_string else [],
efriis344 days ago

To be clear I'm good with this, but could this be a breaking change if someone was relying on content as a str

jacoblee93343 days ago

This wouldn't be getting hit if coerce_content_to_string were true, which it is unless you're passing tools, which in the present version aren't streamed. But it's unnecessary, I'll remove

jacoblee93343 days ago👍 1

Never mind, we need this since it's the first chunk. Otherwise we have to update the core logic again to replace an empty string chunk which seems messy.

Conversation is marked as resolved
Show resolved
libs/partners/anthropic/langchain_anthropic/chat_models.py
11141060 event: anthropic.types.RawMessageStreamEvent,
11151061 *,
11161062 stream_usage: bool = True,
1063
 coerce_content_to_string: bool = False,
efriis344 days ago

why does this need a default value?

efriis344 days ago

If internal helper methods, might be more stable if we remove default, and has to be passed in. Don't feel super strongly though

jacoblee93343 days ago

Sure

jacoblee93 Merge branch 'master' of https://github.com/langchain-ai/langchain in…
9f0ec4f4
jacoblee93 Avoid deprecated Pydantic method, address feedback
f7bb12c0
efriis
efriis approved these changes on 2024-06-14
dosubot dosubot added lgtm
efriis efriis merged 181a6198 into master 343 days ago
efriis efriis deleted the jacob/anthropic_streaming_tools branch 343 days ago

Login to write a write a comment.

Login via GitHub

Reviewers
Assignees
Labels
lgtm 🤖:improvement size:L partner 🔌: anthropic
Milestone
No milestone
Terms Of Use Privacy Policy Refund Policy Impressum