Skip to content

[Fix][Core] Fix SeaTunnel CLI reasoning replay for tool calls#10902

Open
yzeng1618 wants to merge 2 commits into
apache:devfrom
yzeng1618:dev-seatunnel-cli
Open

[Fix][Core] Fix SeaTunnel CLI reasoning replay for tool calls#10902
yzeng1618 wants to merge 2 commits into
apache:devfrom
yzeng1618:dev-seatunnel-cli

Conversation

@yzeng1618
Copy link
Copy Markdown
Collaborator

Purpose of this pull request

企业微信截图_17791753088204

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_content to be passed back on assistant tool-call replay messages. Previously, SeaTunnel CLI only sent reasoning_content when non-empty reasoning text was collected. For some compatible APIs, the next tool-call request could fail with:

The reasoning_content in the thinking mode must be passed back to the API.

This patch always includes reasoning_content for OpenAI-compatible assistant tool-call replay messages when OPENAI_ECHO_REASONING_CONTENT is enabled.

This pull request also handles planner responses where tables is null. The CLI now normalizes tables: null to 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_content replay.

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

@yzeng1618 yzeng1618 requested a review from SEZ9 May 19, 2026 07:30
Copy link
Copy Markdown
Contributor

@DanielLeens DanielLeens left a comment

Choose a reason for hiding this comment

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

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.py now preserves OpenAI reasoning_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_content too 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_use branch, the latest code sets msg_dict["reasoning_content"] whenever OPENAI_ECHO_REASONING_CONTENT is enabled, even if reasoning_parts is 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 sends reasoning_content: "".
  • 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_content in the has_tool_use branch when reasoning_parts is non-empty, exactly like the regular assistant branch already does.

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.
  • 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.

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