[Fix][Core] Fix SeaTunnel CLI reasoning replay for tool calls#10902
[Fix][Core] Fix SeaTunnel CLI reasoning replay for tool calls#10902yzeng1618 wants to merge 2 commits into
Conversation
DanielLeens
left a comment
There was a problem hiding this comment.
Thanks for working on this. I reviewed the full diff on the latest head (67ad9dba6c81bdd2324c17551ec27257426509f2), traced the planner/config/fix tool-call loop locally, and the overall direction makes sense. Preserving provider-specific reasoning/thinking state across tool calls is the right fix. That said, I still see one blocker in the current OpenAI replay path.
What this PR fixes
- User pain: after a tool call, models that require reasoning-state replay can reject the next request because the CLI used to drop that provider-specific state.
- Fix approach:
llm_provider.pynow preserves OpenAIreasoning_content, Anthropic thinking/signature blocks, and Bedrock reasoning blocks in the internal message format and replays them on the next round. - One-line summary: the abstraction is moving in the right direction, but the latest implementation still replays
reasoning_contenttoo aggressively on the normal OpenAI tool-call path.
Runtime / execution chain I checked
Orchestrator tool loop
-> client.chat_stream(...)
-> LLMProvider.collect_stream(events) reconstructs assistant_content
-> assistant_content is appended to conversation history
-> next round calls OpenAIProvider._to_openai_messages(...)
-> assistant tool-call history is serialized back to OpenAI-compatible chat messages
Issue 1: empty reasoning_content is replayed unconditionally for assistant tool-call history
- Location:
seatunnel-cli/seatunnel_cli/llm_provider.py:795-819 - Why this is a problem:
- In the
has_tool_usebranch, the latest code setsmsg_dict["reasoning_content"]wheneverOPENAI_ECHO_REASONING_CONTENTis enabled, even ifreasoning_partsis empty. - That is inconsistent with the regular assistant-text branch in the same file (
seatunnel-cli/seatunnel_cli/llm_provider.py:831-839), which only replays the field when reasoning content actually exists. - So on a normal tool-call session where the previous assistant message did not return
reasoning_content, the next request still sendsreasoning_content: "".
- In the
- Risk:
- This is on the main tool-call path, not a corner case.
- Strict OpenAI-compatible endpoints can reject the next request because the history now contains a non-standard / empty reasoning field that was never present in the original assistant output.
- Better fix:
- Only add
reasoning_contentin thehas_tool_usebranch whenreasoning_partsis non-empty, exactly like the regular assistant branch already does.
- Only add
Issue 2: the new replay logic still has no regression tests
- Location:
seatunnel-cli/seatunnel_cli/llm_provider.py:168-309, 766-891 - Why this matters:
- This PR changes the core reconstruction / replay logic for three providers, but there is still no automated coverage for:
- tool-call history with no reasoning blocks,
- tool-call history with real reasoning blocks,
- Anthropic / Bedrock reasoning round-trips.
- This PR changes the core reconstruction / replay logic for three providers, but there is still no automated coverage for:
- I would strongly recommend adding a few focused unit tests around
collect_stream(),_to_openai_messages(), and_from_openai_response()so this does not regress again.
Merge conclusion
- Conclusion: fix before merge.
- Blocking item: Issue 1.
- Non-blocking follow-up: add the regression tests from Issue 2.
I like the general shape of the fix, and the error-message improvements in cli.py are helpful. Once the OpenAI tool-call branch only replays reasoning_content when it actually exists, I am happy to re-review quickly.
Purpose of this pull request
This pull request fixes SeaTunnel CLI behavior when using OpenAI-compatible reasoning models and planner-generated structured plans.
The main fix is for OpenAI-compatible thinking models that require
reasoning_contentto be passed back on assistant tool-call replay messages. Previously, SeaTunnel CLI only sentreasoning_contentwhen non-empty reasoning text was collected. For some compatible APIs, the next tool-call request could fail with:This patch always includes
reasoning_contentfor OpenAI-compatible assistant tool-call replay messages whenOPENAI_ECHO_REASONING_CONTENTis enabled.This pull request also handles planner responses where
tablesisnull. The CLI now normalizestables: nullto an empty list, which avoids runtime failures for sources that do not require table names, such as FakeSource.Does this PR introduce any user-facing change?
Yes.
For SeaTunnel CLI users using OpenAI-compatible reasoning models, multi-turn tool-call conversations no longer fail with a 400 error requiring
reasoning_contentreplay.For planner-generated structured plans with
tables: null, the CLI now treats the value as an empty table list instead of failing during pipeline expansion.This change affects SeaTunnel CLI behavior on the dev branch.
How was this patch tested?
This patch was tested with SeaTunnel CLI unit tests
Check list
New License Guide
incompatible-changes.mdto describe the incompatibility caused by this PR.