fix: preserve reasoning_content for DeepSeek V4 thinking mode#611
fix: preserve reasoning_content for DeepSeek V4 thinking mode#611octo-patch wants to merge 4 commits intoTauricResearch:mainfrom
Conversation
…Research#572) Conditionally include both the memory content and the surrounding instruction only when past_memories is non-empty. Prevents agents from hallucinating past lessons when no memories have been stored yet. Affected agents: Bull Researcher, Bear Researcher, Research Manager, Portfolio Manager, Trader.
…truction Addresses gemini-code-assist review feedback: - All five files (portfolio_manager, research_manager, bear/bull researchers, trader) now call .strip() on past_memory_str before injecting it into prompts, removing the trailing '\n\n' artifacts from the construction loop. - portfolio_manager now also makes the 'past reflections' phrasing in the Investment Thesis instruction conditional on past_memories being non-empty, so an empty memory section can't trigger hallucinated past reflections.
…auricResearch#599) DeepSeek V4 models in thinking mode require reasoning_content to be echoed back in subsequent API calls. LangChain's _convert_dict_to_message drops this field when parsing responses, and _convert_message_to_dict omits it when serializing outgoing messages, causing a 400 error. NormalizedChatOpenAI now overrides two methods: - _create_chat_result: captures reasoning_content from the raw response and stores it in AIMessage.additional_kwargs - _get_request_payload: injects reasoning_content back into the message dicts sent to the API on subsequent turns This is a client-side shim until langchain-openai adds native support. Co-Authored-By: Octopus <liyuan851277048@icloud.com>
![gemini-code-assist[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the prompt generation logic across multiple agent nodes—including the Portfolio Manager, Research Manager, Bear/Bull Researchers, and Trader—to conditionally include past memory reflections, ensuring cleaner prompts when no memories are available. It also updates the NormalizedChatOpenAI client to support DeepSeek V4's thinking mode by preserving and echoing reasoning_content. Review feedback focuses on simplifying loops by removing unused enumeration indices in several files and refining the reasoning_content check to explicitly handle potential empty strings to avoid API errors.
| choices = response_dict.get("choices") or [] | ||
| for gen, choice in zip(result.generations, choices): | ||
| reasoning = choice.get("message", {}).get("reasoning_content") | ||
| if reasoning and isinstance(gen.message, AIMessage): |
There was a problem hiding this comment.
The current check if reasoning will skip empty strings. While DeepSeek usually returns a non-empty string when reasoning is present, the API requirement is to echo the field back if it was provided. Using if reasoning is not None is safer to ensure that even an empty reasoning string (if ever returned) is preserved and echoed back, avoiding potential 400 errors.
| if reasoning and isinstance(gen.message, AIMessage): | |
| if reasoning is not None and isinstance(gen.message, AIMessage): |
| @@ -22,6 +22,17 @@ def portfolio_manager_node(state) -> dict: | |||
| for i, rec in enumerate(past_memories, 1): | |||
There was a problem hiding this comment.
The loop index i is defined but never used in the loop body. You can simplify this by iterating directly over the list.
| for i, rec in enumerate(past_memories, 1): | |
| for rec in past_memories: |
| @@ -20,6 +20,12 @@ def research_manager_node(state) -> dict: | |||
| for i, rec in enumerate(past_memories, 1): | |||
There was a problem hiding this comment.
The loop index i is defined but never used in the loop body. You can simplify this by iterating directly over the list.
| for i, rec in enumerate(past_memories, 1): | |
| for rec in past_memories: |
| @@ -19,6 +19,17 @@ def bear_node(state) -> dict: | |||
| for i, rec in enumerate(past_memories, 1): | |||
There was a problem hiding this comment.
The loop index i is defined but never used in the loop body. You can simplify this by iterating directly over the list.
| for i, rec in enumerate(past_memories, 1): | |
| for rec in past_memories: |
| @@ -19,6 +19,17 @@ def bull_node(state) -> dict: | |||
| for i, rec in enumerate(past_memories, 1): | |||
There was a problem hiding this comment.
The loop index i is defined but never used in the loop body. You can simplify this by iterating directly over the list.
| for i, rec in enumerate(past_memories, 1): | |
| for rec in past_memories: |
| @@ -20,8 +20,12 @@ def trader_node(state, name): | |||
| if past_memories: | |||
| for i, rec in enumerate(past_memories, 1): | |||
There was a problem hiding this comment.
The loop index i is defined but never used in the loop body. You can simplify this by iterating directly over the list.
| for i, rec in enumerate(past_memories, 1): | |
| for rec in past_memories: |
Gemini review flagged that `if reasoning` would silently drop reasoning_content when it equals an empty string. DeepSeek's API requires the field be echoed back verbatim if it was provided, so checking `is not None` is the safer guard. Co-Authored-By: Octopus <liyuan851277048@icloud.com>
|
Addressed in 67582de — switched to The unused |
…class Resolves #599: thinking-mode models require reasoning_content to be echoed back across turns; multi-turn agent runs failed with HTTP 400. The fix isolates DeepSeek's quirks (reasoning_content round-trip and the deepseek-reasoner no-tool_choice limitation) into a subclass so the general OpenAI-compatible client stays untouched. Adds DeepSeek V4 Pro/Flash to the catalog. 9 new tests; rationale documented in the class docstrings. Design adapted from #600; #611 closed in favour of this approach.
|
Thanks @octo-patch for the careful contribution! Closing in favour of the design that landed in 7e9e7b8. The reasoning_content propagation idea is the same; we used the alternative implementation because the agent-file changes in this PR overlap with the v0.2.4 memory redesign (the past_memories prompt branches were removed in #579/ebd2e12 and reintroducing them here would regress that work). Appreciate the contribution. |
3 participants
Fixes #599
Problem
When using DeepSeek V4 models (
deepseek-v4-flash,deepseek-v4-pro) with thinking mode enabled (e.g.reasoning_effort: "high"), subsequent API calls fail with:DeepSeek's API requires that when a response includes
reasoning_content, that field must be echoed back in the assistant message on the next turn. LangChain's current implementation drops this field in both directions:_convert_dict_to_messagedoes not capturereasoning_contentintoAIMessage.additional_kwargs_convert_message_to_dictdoes not includereasoning_contentwhen serializing assistant messagesSolution
Override two methods in
NormalizedChatOpenAI:_create_chat_result— after the parent converts the raw response, extractsreasoning_contentfrom each choice's message dict and stores it inAIMessage.additional_kwargs["reasoning_content"]._get_request_payload— after the parent builds the request payload, injectsreasoning_contentback into any assistant message dict where it is present inadditional_kwargsbut missing from the converted dict.This is a minimal, targeted shim that does not affect other providers. The default DeepSeek V3 model (no thinking mode) is unaffected since
reasoning_contentis absent from its responses.Testing
tests/test_model_validation.py)_create_chat_resultand_get_request_payload