Skip to content

[fix] Fix ReActAgent failure when output schema is null.#838

Open
wenjin272 wants to merge 1 commit into
apache:release-0.2from
wenjin272:fix-react-0.2
Open

[fix] Fix ReActAgent failure when output schema is null.#838
wenjin272 wants to merge 1 commit into
apache:release-0.2from
wenjin272:fix-react-0.2

Conversation

@wenjin272

@wenjin272 wenjin272 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Purpose of change

Fix ReActAgent failure when output schema is null

Tests

it

API

no

Documentation

  • doc-needed
  • doc-not-needed
  • doc-included

Backport of apache#837 to release-0.2.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added doc-not-needed Your PR changes do not impact docs fixVersion/0.2.2 priority/major Default priority of the PR or issue. labels Jun 11, 2026
@wenjin272 wenjin272 added bug [Issue Type] Something isn't working as expected. and removed priority/major Default priority of the PR or issue. labels Jun 11, 2026
@github-actions github-actions Bot added doc-not-needed Your PR changes do not impact docs and removed doc-not-needed Your PR changes do not impact docs labels Jun 11, 2026

@weiqingy weiqingy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for tracking this down — omitting the output_schema key when it's null is the right fix (the old null value was what ActionJsonSerializer choked on, and an absent key reads back as null/None just the same), and nice that the Java and Python sides stay symmetric. One question on the new Java test inline.

.apply(agent)
.toDataStream();

out.print();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This test passes as long as env.execute() doesn't throw, so it covers "the plan serializes and the job runs" — which is the crash this PR fixes — but it doesn't pin the contract the no-schema path establishes: that the output comes back as a String. The Python counterpart goes a step further with assert len(output_list) == 1 and assert isinstance(output_list[0]["0001"], str), and the schema-case Java test routes through checkResult(...) for an exact value check. Would it be worth collecting the output here too — collectAsync() like the schema test, then asserting hasNext() and that the value is a String — so a regression that emitted nothing or wrongly took the STRUCTURED_OUTPUT branch in stopAction would fail on the Java side as well? Or is the intent to keep the Java e2e deliberately lighter than the Python one?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I agree that the Java test also needs to validate the results, and I will update this PR accordingly.

Additionally, since this fix has already been merged into the main branch, I will open a separate PR later to update the Java tests on the main branch as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug [Issue Type] Something isn't working as expected. doc-not-needed Your PR changes do not impact docs fixVersion/0.2.2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants