weiqingy opened a new pull request, #1040: URL: https://github.com/apache/flink-agents/pull/1040
Linked issue: #936 ### Purpose of change Case 2 of #936. `finish_reason` was never read in either language, so a completion truncated by a token limit came back as ordinary partial content with nothing marking it incomplete. Under structured output the partial content went straight to a JSON parser, so the caller got an error naming the parser rather than the truncation. When the truncated content happened to still be valid JSON, no error was raised at all and the caller silently received wrong structured output. Cases 1 and 3 shipped in #952 and #989. This closes the last one. The change is in three parts. The OpenAI-family connections now record the provider's reason on the response as `finish_reason` in the message metadata, in both languages. Unrecognized values are stored verbatim and an absent reason writes no key, so OpenAI-compatible servers that omit the field are unaffected. The shared structured-output path inspects that reason before parsing. `length` and `content_filter` fail with the cause named; `stop`, `tool_calls`, an unrecognized value and an absent key all parse as before. Providers that never record a reason are therefore unaffected. Non-structured calls are untouched, so a truncated answer is still returned and the caller decides what to do with it. Python's outbound converter needed fixing first. It merged the whole `extra_args` dict into outbound system, user and assistant messages, and because the chat action stores the assistant response with its metadata and replays it on the next turn of a tool loop, `model_name`, `promptTokens`, `completionTokens` and `structured_output` were being sent back to the provider as top-level message fields. `structured_output` holds a Pydantic model or a pyflink `Row`, which is not a legal message field at all. Each role now sends only the fields OpenAI defines for it, matching what the Java converter already produced. Without this, adding `finish_reason` would have widened that leak. Three things worth flagging for review. The Java capture reads the reason through `_finishReason().asKnown()` rather than `finishReason()`. The plain accessor resolves through `JsonField.getRequired` and throws `OpenAIInvalidDataException` when the member is absent or JSON-null. `VLLMChatModelConnection` extends `OpenAICompletionsConnection`, so OpenAI-compatible servers that omit the member use this path, and the plain accessor would have failed calls that previously succeeded. The check runs before the parser execution is reported rather than inside the parse. The reporting wrapper records any exception from the parse as `model_output_parse_error`, so gating inside it would report a truncation as a parse failure, which is the same misleading diagnosis this change removes. Rejecting beforehand records no parser execution, which is accurate because none was attempted. The trade is that a truncation no longer produces a `PARSER` failure report; the action-level failure is still reported. Java's structured-output result now carries the inbound metadata forward instead of being rebuilt from an empty map. Previously it discarded whatever the connection had recorded, so Java dropped `finish_reason` exactly when an output schema was in play while Python kept it. This also means `model_name` and the token counts now reach Java callers of structured output, as they always have in Python. Two behavior changes callers may notice. A Python caller can no longer put an arbitrary key in `extra_args` and have it forwarded to the provider; Java never allowed this, and nothing in the repository relies on it. And truncated content that still parses previously produced wrong output and now raises, so callers using the `RETRY` strategy will see such responses retried. The Responses API is out of scope: it reports `status` and `incomplete_details.reason`, a different vocabulary. Anthropic, Ollama, Gemini, Bedrock and Tongyi record no reason and are unaffected, and will pick the behavior up if they later write the same key. ### Tests `./tools/build.sh` and `./tools/ut.sh` both green: 33/33 modules, Java 1592 run with 0 failures and 0 errors, Python 938 passed. Each of the five commits also builds and passes independently, with no commit depending on a later one. Coverage was driven by mutation testing rather than by line count, and every mutant in scope is killed in both languages. The cases that earned dedicated tests are the ones a plausible implementation gets wrong: swapping in the throwing SDK accessor, moving either capture inside the token-usage guard, moving the gate inside the reporting wrapper, dropping either terminal reason, and reinstating the outbound blanket merge. Two test properties are load-bearing and easy to lose in a later refactor. The throwing tests use valid JSON, because truncated JSON raises `JSONDecodeError` or `ValidationError` on its own and a test using malformed content would pass with no gate at all. And one test asserts that a rejected response produces no parser execution report, which is the only thing preventing the gate from drifting back inside the wrapper where every other test still passes. Both languages were also driven through identical inputs end to end to confirm they agree, including that neither sends `finish_reason`, `model_name`, `promptTokens` or `completionTokens` to a provider. The two error messages are byte-identical across languages; only the exception type differs. ### API No public API change. `finish_reason` is a metadata key, not a new field, and no new exception type or problem category is introduced. Failures reuse `ValueError` in Python and `IllegalStateException` in Java, both already used for this class of failure in their respective packages. Because `json.JSONDecodeError` is a `ValueError` subclass, Python callers already catching today's truncation symptom keep catching it and simply get a clearer message. Javadoc on the public `chat` overrides documents the metadata contract, matching the existing Python docstring. ### Documentation - [ ] `doc-needed` - [x] `doc-not-needed` - [ ] `doc-included` ### Was this patch authored or co-authored using generative AI tooling? - [x] Yes - [ ] No Generated-by: Claude Code 2.1.234 (Claude Opus 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]
