Skip to content

feat(agent): forward model settings and recover from thinking stalls [PC-4672] - #1017

Open
tudormatei1 wants to merge 8 commits into
mainfrom
feat/model-specific-settings
Open

feat(agent): forward model settings and recover from thinking stalls [PC-4672]#1017
tudormatei1 wants to merge 8 commits into
mainfrom
feat/model-specific-settings

Conversation

@tudormatei1

@tudormatei1 tudormatei1 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Anthropic won't honor a forced tool_choice while extended or adaptive thinking is on, so a thinking model can answer in plain text and never call end_execution. The old consecutive-thinking counter turned that into a hard THINKING_LIMIT_EXCEEDED on the first occurrence. Now one tool-less turn is tolerated and retried with thinking off and the tool call forced (react/forced_extraction.py), which every provider honors; only a second stall fails, with the same error code.

Stall accounting moved out of the router and into the LLM node, because the router can't tell whether forcing actually survived on the wire — the Bedrock handlers silently downgrade any -> auto under thinking. A tool-less turn with content now always loops back to the agent. AgentGraphConfig.thinking_messages_limit is no longer read anywhere; I left the field in place so existing agent.json files don't break, but it's dead — say the word if you'd rather deprecate it properly.

New chat/thinking.py holds the transport-specific knowledge in one place (native thinking attribute, model_kwargs for Bedrock Invoke, additional_model_request_fields for Converse) so the payload handlers and the ReAct loop stop each reimplementing it. Replaying reasoning blocks on a thinking-off call 400s, so the extraction retry strips them and appends a user turn if the history ends on an AI message.

Two smaller things ride along. get_chat_model gains a model_settings passthrough for the client-side work in uipath-langchain-client 1.18.0 (dependency bumped), with a warning when the legacy clients would silently drop it. And exceptions/licensing.py folded into exceptions/llm.py: provider HTTP errors are now re-raised with from None and only the gateway detail, since the vendor's passthrough message can echo request content and shouldn't reach the run record or App Insights; 5xx also categorizes as SYSTEM instead of UNKNOWN.

Verified with the 184 tests across the touched files.

@tudormatei1
tudormatei1 marked this pull request as ready for review August 18, 2026 07:42
Copilot AI lite review requested due to automatic review settings August 18, 2026 07:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR improves agent/tool-call reliability under Anthropic “thinking” modes across transports, adds safer provider HTTP error normalization, and introduces model_settings forwarding for the new chat model factory path.

Changes:

  • Add shared “thinking” utilities and Bedrock handler logic to downgrade forced tool-choice when thinking is active, plus a forced-extraction retry path in the ReAct LLM node.
  • Refactor ReAct routing/stall accounting from router-level “thinking message limits” to LLM-node stall handling, with new/updated tests.
  • Add model_settings parameter to get_chat_model, warn when ignored on legacy clients, and bump uipath-langchain-client dependency.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/uipath_langchain/chat/thinking.py Introduces shared helpers for detecting/stripping thinking and reasoning blocks.
src/uipath_langchain/chat/handlers/bedrock.py Uses shared predicate to downgrade forced tool choice under thinking on Bedrock.
src/uipath_langchain/chat/chat_model_factory.py Adds model_settings arg, forwards to new factory, warns on legacy path.
src/uipath_langchain/agent/react/llm_node.py Moves stall handling into LLM node; adds forced-extraction retry and deterministic failure.
src/uipath_langchain/agent/react/router.py Removes thinking-limit enforcement; routes tool-less content back to agent.
src/uipath_langchain/agent/react/utils.py Renames & documents stall counter as “tool-less turns”.
src/uipath_langchain/agent/react/forced_extraction.py New helper to strip thinking + reasoning blocks and append trailing user turn.
src/uipath_langchain/agent/exceptions/llm.py Consolidates provider HTTP error mapping and adds category/status handling.
src/uipath_langchain/agent/exceptions/licensing.py Removes legacy provider HTTP error mapping module.
tests/... Adds/updates tests for model_settings dispatch, Bedrock downgrade behavior, forced extraction, routing changes, and HTTP error redaction.
pyproject.toml Version bump and dependency range bump for uipath-langchain-client.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/uipath_langchain/agent/exceptions/llm.py
Comment thread src/uipath_langchain/chat/thinking.py Outdated
Comment thread src/uipath_langchain/chat/thinking.py
Comment thread src/uipath_langchain/agent/react/llm_node.py
Comment thread src/uipath_langchain/agent/react/forced_extraction.py Outdated
Comment thread src/uipath_langchain/chat/chat_model_factory.py
@tudormatei1 tudormatei1 changed the title feat(agent): forward model settings and skip forced tool_choice under Claude thinking [PC-4672] feat(agent): forward model settings and recover from thinking stalls [PC-4672] Aug 18, 2026
@tudormatei1
tudormatei1 force-pushed the feat/model-specific-settings branch from 29cce8a to 9b88d18 Compare August 18, 2026 10:12
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 90%)

See analysis details on SonarQube Cloud

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.

2 participants