feat(agents): support expanded reasoning response in agent chat#2634
feat(agents): support expanded reasoning response in agent chat#2634ks93 wants to merge 5 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2634 +/- ##
==========================================
- Coverage 93.08% 93.04% -0.04%
==========================================
Files 486 486
Lines 49671 49728 +57
==========================================
+ Hits 46235 46271 +36
- Misses 3436 3457 +21
🚀 New features to boost your workflow:
|
884f01b to
84f5b4d
Compare
Add the `data` field to `AgentReasoningItem` to surface tool-call details from the agent's reasoning trace. Introduces `ToolCallDetail`, `ReasoningDataItem`, `ToolCallReasoningDataItem`, and `UnknownReasoningDataItem` following the same type-dispatch pattern used by `MessageContent`.
84f5b4d to
bda3637
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces new data classes to support tool call reasoning data in agent chat responses, including ToolCallDetail and a ReasoningDataItem hierarchy. Review feedback focuses on reducing redundant serialization logic by leveraging base class functionality and ensuring that data loading methods are robust enough to handle both camelCase and snake_case keys for better compatibility.
- Remove manual ToolCallDetail.dump (base class handles camelCase conversion) - Add dump to ReasoningDataItem base for consistency with MessageContent pattern - Handle toolCall/tool_call key in ToolCallReasoningDataItem._load_item
haakonvt
left a comment
There was a problem hiding this comment.
A few comments, mostly LGTM
|
@nithinb ping 😄 |
nithinb
left a comment
There was a problem hiding this comment.
I had a few comments, feel free to reach out once the changes are made.
| def _load(cls, data: dict[str, Any]) -> AgentReasoningItem: | ||
| content = [MessageContent._load(item) for item in data.get("content", [])] | ||
| return cls(content=content) | ||
| data_items = [ReasoningDataItem._load(item) for item in data.get("data", [])] or None |
There was a problem hiding this comment.
Can data.get("data", []) be an empty list?
Because this silently converts an empty list to None. If it doesn't matter, I think a comment would be ideal in such scenarios
| data = data.copy() | ||
| item_type = data.pop("type") | ||
| return cls(type=item_type, data=data) |
There was a problem hiding this comment.
nit and not blocking but generally it is better to store data.copy() in a separate variable instead of storing it in the same variable.
Like same name used for parameter and copy of the parameter.
| reasoning_item = agent_msg.reasoning[0] | ||
| assert isinstance(reasoning_item, AgentReasoningItem) | ||
| assert isinstance(reasoning_item.content[0], TextContent) | ||
| assert reasoning_item.data is not None | ||
| assert isinstance(reasoning_item.data[0], ToolCallReasoningDataItem) | ||
|
|
There was a problem hiding this comment.
I think you should also add validation of actual data in reasoning_item.data and reasoning_item.content
Summary
datafield toAgentReasoningItem, which the Agents API now returns to expose tool-call details from the agent's reasoning traceToolCallDetail,ReasoningDataItem,ToolCallReasoningDataItem, andUnknownReasoningDataItemfollowing the same type-dispatch pattern asMessageContent/TextContent/UnknownContentcognite.client.data_classes.agentsAPI reference: https://api-docs.cognite.com/20230101-beta/tag/Agents/operation/agent_session_ai_agents_chat_post#!c=200&path=response/2/messages/reasoning/data&t=response
Test plan
Test manually locally.
pytest tests/tests_unit/test_api/test_agents_chat.py -v-- all 11 tests passdata=Noneis preserved when field is absent from the API response