Skip to content

fix(llm): prevent orphaned tool_calls and missing type field in continue-gen path#908

Open
lobstersyrup wants to merge 2 commits intomodelscope:mainfrom
lobstersyrup:fix/format-tools-in-continue-gen
Open

fix(llm): prevent orphaned tool_calls and missing type field in continue-gen path#908
lobstersyrup wants to merge 2 commits intomodelscope:mainfrom
lobstersyrup:fix/format-tools-in-continue-gen

Conversation

@lobstersyrup
Copy link
Copy Markdown

@lobstersyrup lobstersyrup commented May 3, 2026

Fix 1: Missing type field in continue-gen tool calls

Problem: _call_llm_for_continue_gen passes raw Tool TypedDicts directly to _call_llm without formatting first. The Tool TypedDict has no type field — it gets added by format_tools(). When a response hits finish_reason: length and enters the continue-generation path, the unformatted tools cause:

openai.BadRequestError: Error code: 400 - tools[0]: missing field `type`

Fix: Added self.format_tools(tools) in _call_llm_for_continue_gen in both openai_llm.py and deepseek_llm.py.

Fix 2: Orphaned tool_calls causing "insufficient tool messages" error (fixes #909)

Problem: When finish_reason: length fires with tool_calls present in the truncated response, _continue_generate and _stream_continue_generate append the partial assistant message (containing tool_calls) to the message history, then immediately call the LLM for continuation without executing the tools. The resulting messages list has assistant tool_calls but no corresponding tool responses, causing:

openai.BadRequestError: insufficient tool messages following tool_calls message

This causes sub-agents (e.g., Reporter) to crash after 3 retries, leading to infinite re-spawn loops.

Fix: Return early when tool_calls are present, letting the normal step() tool-execution loop handle the truncated message:

  • _continue_generate (line 526): if new_message.tool_calls: return new_message
  • _stream_continue_generate (line 346): add not message.tool_calls and guard before the finish_reason continuation check

Testing

Tested both fixes with deepseek-v4-pro on the deep_research/v2 pipeline. Without the fixes, the pipeline reliably hits both errors. With both fixes applied, the pipeline completes without errors.

_call_llm_for_continue_gen was passing raw Tool dicts directly to
_call_llm without calling format_tools first. The raw Tool TypedDict
lacks the 'type': 'function' field required by OpenAI-compatible APIs.

This caused 400 errors ('tools[0]: missing field type') from providers
that strictly validate tool definitions (e.g., DeepSeek), particularly
when long responses triggered finish_reason=length and entered the
continue-generation path.

Fixed in both OpenAI._call_llm_for_continue_gen and
DeepSeek._call_llm_for_continue_gen to call self.format_tools(tools),
matching the existing pattern in generate().
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the _call_llm_for_continue_gen method in both DeepSeekLLM and OpenAILLM to ensure tools are properly formatted before the LLM call. While the tool formatting fix is correct, the implementation in DeepSeekLLM contains several critical issues: a logic error where the stop sequences are lost because list.append() returns None, a potential AttributeError due to a naming mismatch in the message formatting call, and a missing update to the API call counter.

stop = kwargs.pop('stop', []).append('```')
return self._call_llm(
messages=messages, tools=tools, stop=stop, **kwargs)
messages=messages, tools=self.format_tools(tools), stop=stop, **kwargs)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

While adding self.format_tools(tools) correctly addresses the tool formatting issue, this method contains several other critical bugs that will likely prevent it from functioning as intended:

  1. AttributeError: Line 37 calls self.format_input_message(messages), but the base class method is named _format_input_message (with a leading underscore). This will cause a runtime error.
  2. Logic Error: Line 38 incorrectly sets the stop variable to None because list.append() operates in-place and returns None. Consequently, the stop=stop argument passed on line 40 will always be None, effectively losing all intended stop sequences.
  3. Redundancy: The _call_llm method already handles message formatting internally (via _format_input_message), making the call on line 37 unnecessary even if the name were correct.
  4. Missing State Update: Unlike the base class implementation in OpenAI, this override fails to increment messages[-1].api_calls, which may affect tracking or logic dependent on the number of API attempts.

I recommend refactoring this method to address these issues alongside the tool formatting fix.

@lobstersyrup
Copy link
Copy Markdown
Author

Thanks for the thorough review. All four findings are technically correct, but they identify pre-existing bugs in a class that's dead code:

The DeepSeek class isn't registered in model_mapping.py's all_services_mapping, so LLM.from_config() never instantiates it. Its __init__ requires deepseek_base_url and deepseek_api_key config keys that no standard config provides — users can't reach it through the normal config path. It has no unit tests and isn't referenced by any example or project config in the repo.

Our one-line change (adding self.format_tools(tools)) is consistent with the existing pattern in OpenAI._call_llm_for_continue_gen and is harmless belt-and-suspenders — it would work correctly if the class were ever made reachable.

The real fix this PR delivers is in openai_llm.py line 502, which actually executes. The deepseek_llm.py change is included for completeness.

When finish_reason=length with tool_calls present, _continue_generate
and _stream_continue_generate append the partial message (with
tool_calls) to history then immediately call the LLM for continuation
without executing the tools. The resulting messages list has assistant
tool_calls with no corresponding tool responses, causing:

  openai.BadRequestError: insufficient tool messages following
  tool_calls message

Fix: return early when tool_calls are present, letting the normal
step() tool-execution loop handle the message.

- _continue_generate: return new_message immediately if tool_calls
- _stream_continue_generate: skip continuation if tool_calls present

Fixes modelscope#909
@lobstersyrup lobstersyrup changed the title fix(llm): format tools in continue-gen path to include type field fix(llm): prevent orphaned tool_calls and missing type field in continue-gen path May 3, 2026
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.

[framework] _continue_generate corrupts conversation when truncated response contains tool_calls

1 participant