Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
| return [ | ||
| message | ||
| if message._content or not message.is_from(ChatRole.ASSISTANT) | ||
| else ChatMessage.from_assistant(text="", meta=message.meta, name=message.name) |
There was a problem hiding this comment.
shall reasoning be carried over?
There was a problem hiding this comment.
I think I'll be going for a different approach, but that shouldn't matter here anyways because if reasoning content is present then if message._content would trigger and we would return the original message.
There was a problem hiding this comment.
But that could return a message with present reasoning, but text=None, right? Not sure if that message is accepted. It seems utils/hf.py#convert_message_to_hf_format doesn't accept it
There was a problem hiding this comment.
Yeah our conversions across providers are not consistent (sometimes for good reason). For openai responses chat generator it does accept a text=None but a resoning content not None.
Would have to look into the hugging face to see if we are being too strict with the conversion.
| # A contentless reply is kept out of the history because converters reject it. Usage and the exit reason | ||
| # still come from the raw replies: the call was billed and may have been truncated. | ||
| exe_context.state.set("messages", _drop_contentless_replies(messages=llm_messages)) | ||
| _record_llm_usage(state=exe_context.state, llm_messages=llm_messages) |
There was a problem hiding this comment.
(sorry if I'm being disruptive)
While this keeps the accumulated/aggregated token usage in the result, it removes the contentless messages and hence their token usage from the result.
I'm actually relying on that (the usage in individual message's meta) to calculate and monitor cost per tool call/message
There was a problem hiding this comment.
No problem thanks for the feedback. Yeah I was also worried about this so I was looking to see if server-side if providers like OpenAI accept content-less messages and we are being too strict. It seems with some initial testing they do accept them so that could be a solution that doesn't require dropping.
There was a problem hiding this comment.
Yeah OpenAI seems to accept them, though Anthropic seems not
There was a problem hiding this comment.
I believe anthropic accepts it if you send [] as the empty content and not ""
Related Issues
Agentcrashes on the next LLM call after a Chat Generator discards a malformed tool call #12541Proposed Changes:
Fixed a crash in
Agenton the LLM call following a reply from which a Chat Generator discarded a malformed tool call. Such a reply has no content parts, which Chat Message converters reject. TheAgentnow leaves it out of the message history, so the next call and any follow-up turn built from the returned messages stay sendable. Its token usage is still recorded, and alengthorcontent_filterfinish reason on it still ends the run.How did you test it?
New tests, and also tested locally that the model does recover.
Notes for the reviewer
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:and added!in case the PR includes breaking changes.