sangkyoonnam opened a new pull request, #1180:
URL: https://github.com/apache/flink-agents/pull/1180
Linked issue: #1179
### Purpose of change
#### User-visible outcome
A textual failure that Python's tool action emits in a `ToolResponseEvent`
now reads on the Java side as `ToolResponse.error(...)` carrying the text
Python shows the model, and `toString()` no longer throws for such events.
Previously every non-map response became `ToolResponse.success(v)`, so a failed
Python tool call looked like a success whose result was the failure text, and
`toString()` threw `NullPointerException` because Python events carry no
`timestamp`.
#### Intent
Python records a failed call as the text it shows the model in `responses`,
with `success[id]` false and the diagnostic in `error[id]`. The Java normalizer
ignored `success`, so the two runtimes disagreed on what the same event means.
#### Runtime flow
`ToolResponseEvent`'s JSON constructor, reached from `fromEvent`, normalizes
`responses`. A `ToolResponse` or a map is handled as before. For any other
value it now checks `success[id]`: when that is `false`, it builds
`ToolResponse.error(String.valueOf(value))`; otherwise it wraps the value in
`ToolResponse.success(v)` as before. During normalization a `success` attribute
that isn't a map is treated as empty; the attribute itself is left as is.
`toString()` prints the raw `success` and `timestamp` attributes instead of
calling `getSuccess()` and `getTimestamp()`.
#### Key decisions
- Use the response value, not `error[id]`, as the error text. Python's chat
action sends `str(response)` to the model, and Java's `ChatModelAction` sends
`getError()` for a failed call, so this keeps what the model sees the same in
both runtimes. The diagnostic stays available through `getError()` on the event.
- Key off `success[id]` rather than inspect the payload, and keep the wire
format, per the tool outcome contract in #956.
- Leave `getTimestamp()` unchanged. Whether Python events should carry a
timestamp is a separate API question; after this change nothing in the
repository calls it for them.
### Behavioral Semantics
#### Interaction decisions
| Response value | `success[id]` | Result |
|---|---|---|
| `ToolResponse` | any | Kept as is (unchanged) |
| Map | any | Converted with Jackson (unchanged) |
| Other | `false` | `ToolResponse.error(String.valueOf(value))` (was
`success(value)`) |
| Other | anything other than `false` | `ToolResponse.success(value)`
(unchanged) |
#### Behavioral contracts
1. A textual response that Python's tool action marked as failed reads as
`isError()`, with the same text as its error.
2. Successful scalar responses from Python still read as
`ToolResponse.success(value)`.
3. `toString()` does not throw when the event has no `timestamp`.
4. `toString()` reports the event's `success` map.
#### Failure behavior
No new failure path. `getTimestamp()` still throws for events without a
`timestamp`, as before.
### Tests
#### Contracts to tests
| Contract | Tests |
|---|---|
| 1 |
`CrossLanguageEventSnapshotTest.pythonToolResponseEventKeepsFailedCallsAsErrors`
|
| 2 |
`CrossLanguageEventSnapshotTest.pythonToolResponseEventRoundTripsScalarResponses`
(pre-existing, updated for the new `error` entry) |
| 3 |
`CrossLanguageEventSnapshotTest.pythonToolResponseEventKeepsFailedCallsAsErrors`
|
| 4 |
`CrossLanguageEventSnapshotTest.pythonToolResponseEventKeepsFailedCallsAsErrors`
|
#### Coverage and what was not verified
The Python snapshot builder now includes a failed call shaped the way
`ToolCallAction` records a raised exception, with different texts in
`responses` and `error`, and the snapshot was regenerated. `mvn -pl
api,plan,runtime test`: 518, 456 and 1142 run, 0 failures. Python `pytest
flink_agents/api`: 568 passed. `tools/lint.sh -c` passes on JDK 11.
Not verified: no end-to-end run with a Java consumer of a Python tool call;
the test reads the event through the cross-language snapshot instead.
Map-valued responses marked as failed keep the existing Jackson conversion.
<details>
<summary>Implementation invariants and supporting evidence</summary>
- Without the `ToolResponseEvent` change the new test fails at the
`isError()` assertion; with only the `toString()` part reverted it fails with
`NullPointerException` from `getTimestamp()`.
- Events emitted by Java's built-in `ToolCallAction` contain `ToolResponse`
objects, so they bypass the new branch.
</details>
### API
#### Compatibility impact
No public signature changes. `fromEvent` on a Python event carrying a
textual failure from Python's tool action now returns an error response instead
of a success; no dependency on the old reading was identified. Java response
normalization is otherwise unchanged. `toString()` output changes for all
events, Java-originated ones included: `success={...}` instead of
`success=true`, and the raw timestamp or `null`.
### Documentation
<!-- Do not remove this section. Check the proper box only. -->
- [ ] `doc-needed` <!-- Your PR changes impact docs -->
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-included` <!-- Your PR already contains the necessary
documentation updates -->
### Was this patch authored or co-authored using generative AI tooling?
<!-- Do not remove this section. Check the proper box only. -->
- [x] Yes
- [ ] No
Generated-by: Claude Code 2.1.283 (Claude Opus 5.5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
