[PR] [api][java] Keep failed Python tool calls as errors in ToolResponseEvent [flink-agents]

["sangkyoonnam (via GitHub)" <gi...@apache.org>]

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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [api][java] Keep failed Python tool calls as errors in ToolResponseEvent [flink-agents]

["wenjin272 (via GitHub)" <gi...@apache.org>]

wenjin272 merged PR #1180:
URL: https://github.com/apache/flink-agents/pull/1180


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org