Skip to content

[integrations][java][python] Apply Azure OpenAI native structured output - #930

Merged
wenjin272 merged 4 commits into
apache:mainfrom
weiqingy:280-pr3-azure-native
Aug 4, 2026
Merged

[integrations][java][python] Apply Azure OpenAI native structured output#930
wenjin272 merged 4 commits into
apache:mainfrom
weiqingy:280-pr3-azure-native

Conversation

@weiqingy

@weiqingy weiqingy commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Linked issue: #280

Purpose of change

The Azure OpenAI connections reject a non-null output_schema, so every caller with a schema stays on the prompt-engineering fallback even when the deployment's backing model supports native structured output. This applies the native path for them, in Java and Python together, on top of the mechanism added in #843.

Two things differ from the OpenAI connection, both forced by how Azure works rather than by preference.

Capability is keyed on model_of_azure_deployment, not model. On Azure the model parameter is the deployment name, an arbitrary string the user chose, so matching an allowlist against it is wrong in both directions: a deployment called prod-chat backed by a capable model would report not-capable, and a deployment called gpt-4o backed by anything would report capable. model_of_azure_deployment is the only input that names the model actually behind the deployment. When it is unset, capability reports false and the caller stays on the prompt path, which is the safe direction.

The native path additionally requires an api_version at or above 2024-08-01. This is a safety gate rather than an optimization. Azure documents 2024-08-01-preview as the first version supporting structured outputs, but I could not determine whether an older version returns an error or silently ignores response_format, and neither SDK does any client-side gating. Never sending below the floor makes both possible behaviors safe: if the service would reject it, the call is never made; if it would silently ignore it, the native path is never taken so the prompt fallback still applies. This matters because the docs currently recommend 2024-02-15-preview and 2024-02-01, both below the floor.

A bare gpt-4o is deliberately absent from the allowlist. Azure reports a model and its version as separate properties, so a bare gpt-4o may be the 2024-05-13 build, which does not support the feature, and no available input disambiguates it. This matches the same exclusion made for the OpenAI connection.

Building the Java request inline left no seam to observe, so chat() is split into a 4-arg override, a private doChat, and package-private buildRequest and toResponse. The split turns a single read of model_of_azure_deployment into two reads that must agree; toResponse makes that testable. Reading the key after the call is equivalent to reading it before because the parameter map is built fresh per call and never retained.

One behavior worth calling out: if a caller supplies their own response_format and the native path is about to write one, the connection now raises instead of silently replacing it. Before this change that combination was unreachable; it produced a raw SDK error on one channel and dropped the caller's value on the other. Passing response_format without a schema is unaffected on every non-native path.

Azure AI Inference (the azureai module) is not included. Native structured output cannot work there at the pinned azure-ai-inference:1.0.0-beta.5: json_schema is rejected below api-version 2024-08-01-preview, but raising ModelServiceVersion to that value makes the endpoint return 404, which is why the SDK pins its own default down. I have recorded the details on #280 so nobody repeats the investigation.

Tests

Both languages pin the same set of behaviors: native applied for a capable model with a POJO/BaseModel schema and an adequate api-version; not applied for an incapable, unknown, or bare-gpt-4o model; not applied for a RowTypeInfo schema or a null schema; not applied below the api-version floor; the capability predicate's true and false cases; the collision raising; and a caller-supplied response_format passing through untouched on every non-native path.

Python: 54 tests pass in flink_agents/integrations/chat_models (non-integration), 447 across api and plan. The new tests are in their own non-integration file, since the existing Azure test module is integration-marked and would be skipped in the unit run.

Java: 58 tests pass in the openai module, spotless:check clean, no compiler warnings. The five pre-existing tests in AzureOpenAIChatModelConnectionTest are unmodified and still pass, including the ones guarding the exception behavior around the refactored region.

The capable-model list and the api-version floor are identical in both languages.

API

No new public type, no new configuration field, and no signature change to an existing method. Java adds a 4-arg chat override of the overload introduced in #843; Python's output_schema parameter already existed and is now honored rather than rejected. A caller that passes no output_schema sees unchanged behavior.

No dependency change in either language, and no build file is touched.

Documentation

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

Was this patch authored or co-authored using generative AI tooling?

  • Yes
  • No

Generated-by: Claude Code (Claude Opus 5)

weiqingy added 2 commits July 26, 2026 22:18
Translate a BaseModel output schema into the provider's native
response_format json-schema parameter instead of rejecting it, so callers
with a schema are no longer forced onto the prompt-engineering fallback.

Capability resolves against model_of_azure_deployment rather than model,
because on Azure the latter is a user-chosen deployment name that carries
no information about the backing model. The native path additionally
requires an api_version at or above Azure's documented 2024-08-01 floor;
below it the schema is never sent, so an older api-version cannot produce
an unconstrained response that the caller mistakes for a structured one.

A caller-supplied response_format that the native path would overwrite now
raises rather than being silently replaced or reaching the SDK as a
duplicate keyword argument.
Translate a POJO output schema into openai-java's native responseFormat
instead of inheriting the rejecting default, mirroring the Python Azure
OpenAI connection: the capable-model allowlist and the api-version floor
are identical in both languages.

Capability is keyed on model_of_azure_deployment rather than model,
because the latter names the Azure deployment and says nothing about the
model behind it. The native path also requires an api_version at or above
2024-08-01, so a schema is never sent to a service version that predates
structured output.

Building the request inline left no seam to observe, so chat() is split
into a 4-arg override, a private doChat, and package-private buildRequest
and toResponse. The split turns one read of model_of_azure_deployment
into two that must agree; toResponse makes that pinnable, and reading the
key after the call is equivalent to reading it before because the
parameter map is built per call and never retained.

A caller-supplied response_format that the native path would overwrite
now raises rather than being silently replaced.
@weiqingy

Copy link
Copy Markdown
Collaborator Author

Implementation Description

As-built description of the final code on this branch, derived by walking git diff origin/main...HEAD hunk by hunk.

1. Runtime flow

Both languages follow the same order. The connection decides everything locally; nothing upstream is consulted.

Python, AzureOpenAIChatModelConnection.chat:

  1. Tools are converted to OpenAI tool specs, unchanged from before.
  2. model is popped from kwargs as the Azure deployment name and required to be non-empty.
  3. model_of_azure_deployment is popped. This is the model backing the deployment.
  4. additional_kwargs is popped, and its keys are checked against the reserved typed fields, unchanged from before.
  5. The native gate is evaluated as three conditions in order: the schema is not None, supports_native_structured_output(model_of_azure_deployment) is true, and _api_version_supports_structured_output() is true.
  6. If the gate passes, _native_response_format(output_schema) is called. It returns None for anything that is not a BaseModel subclass, which is the fourth condition in practice.
  7. If it returned a format, the collision check runs: if response_format is present in kwargs or in additional_kwargs, ValueError is raised. Otherwise kwargs["response_format"] is set.
  8. client.chat.completions.create(...) is called with the deployment name as model, and the response is converted exactly as before.

Java, AzureOpenAIChatModelConnection:

  1. The 3-arg chat now delegates to doChat(..., null). The new 4-arg chat delegates to doChat(..., outputSchema).
  2. doChat calls buildRequest, issues client.chat().completions().create(params), and passes the completion to toResponse.
  3. buildRequest copies the caller's modelParams into a mutable map, then removes model (required non-blank) and model_of_azure_deployment from the copy.
  4. The native gate is evaluated as outputSchema instanceof Class, then supportsNativeStructuredOutput(modelOfAzureDeployment), then apiVersionSupportsStructuredOutput(). If all hold, toNativeResponseFormat(schemaClass) is attached to the builder and the schema's simple name is recorded in a local nativeSchemaName.
  5. temperature, max_tokens, and logprobs are read after that, unchanged from before.
  6. additional_kwargs is removed and checked for reserved typed fields, unchanged. Then, only if nativeSchemaName is non-null, the presence of response_format in additional_kwargs raises IllegalArgumentException. Otherwise every entry is written through as an additional body property, unchanged.
  7. toResponse converts the first choice's message, then reads model_of_azure_deployment from the caller's original map with get rather than remove, and attaches model_name, promptTokens, and completionTokens only when that value is non-blank and completion.usage() is present.

The api-version value is read from a new private final String apiVersion field. Before this change the constructor consumed the value into AzureOpenAIServiceVersion.fromString(...) and discarded it.

2. Behavioral contracts

Each is a checkable claim about the final code.

C1. Native structured output is applied only when all four hold: the schema is non-null, the schema translates (a BaseModel subclass in Python, a Class in Java), the backing model is in the allowlist, and the api-version is at or above 2024-08-01. If any fails, the request is built exactly as it would have been without a schema.

C2. Capability is decided from model_of_azure_deployment, never from model. The request still targets model, the deployment name, as before.

C3. An absent, empty, or unrecognized model_of_azure_deployment reports not-capable, so the request carries no response_format.

C4. A bare gpt-4o reports not-capable, because Azure documents gpt-4o as supported only at versions 2024-08-06 and 2024-11-20 while 2024-05-13 is unsupported, and a model name carries no version.

C5. An api-version whose leading 10 characters sort below 2024-08-01 suppresses the native path. An empty api-version does the same in Python. In Java a null or blank api-version cannot reach the check, because the constructor already rejects it.

C6. A RowTypeInfo schema in Python, and any non-Class schema in Java, suppresses the native path and keeps the prompt-engineering fallback.

C7. A null schema leaves behavior identical to before this change.

C8. Bound tools do not suppress the native path. A request may carry both tools and response_format.

C9. When the native path applies and the caller also supplied response_format, the call raises rather than overwriting or duplicating it. Python checks both kwargs and additional_kwargs. Java checks additional_kwargs, which is its only caller-facing channel, because unrecognized top-level modelParams keys are discarded and never reach the request.

C10. On every path where the native format is not applied, a caller-supplied response_format reaches the provider untouched. In Python the identical object is forwarded, not a copy.

C11. The capability predicate reads no instance state. It is answerable on an instance that was never initialized, where field access would raise.

C12. Response handling does not consume the caller's modelParams. Java reads model_of_azure_deployment with get, so a caller reusing the same map across calls still sees it.

C13. Token metrics (model_name, promptTokens, completionTokens) are attached only when the backing model is non-blank and the completion carries usage. Neither alone is sufficient.

C14. The Java and Python allowlists and floor constant are identical, including ordering of the allowlist entries.

C15. A connection that translates schemas natively is exempt from the cross-connection walk that requires every connection to reject a schema it cannot translate. The exemption is derived from whether supports_native_structured_output is overridden, not from a name list.

3. Key decisions

Capability is keyed on model_of_azure_deployment, not on model. On Azure, model is the deployment name, an arbitrary string chosen by the user. Matching an allowlist against it is wrong in both directions: a deployment named prod-chat backed by a capable model would report not-capable, and a deployment named gpt-4o backed by anything would report capable. The rejected alternative was to reuse the OpenAI connection's rule unchanged, which is exactly that mistake. The cost of this choice is that leaving the optional model_of_azure_deployment unset keeps even a capable deployment on the fallback. That direction of error is the safe one.

Matching is exact, never by prefix. The sibling OpenAI connection matches one model family by prefix. Azure reports a model name and a model version as separate properties, so a name has no version suffix to discriminate on, and prefix matching would classify unsupported versions as capable.

The api-version floor is enforced in the request path, not in the capability predicate. The predicate's argument is a model name and carries no api-version, and the predicate must stay free of instance state (C11). Putting the floor check in the predicate would break that.

The floor is enforced conservatively because the failure mode below it is not documented. Azure documents 2024-08-01-preview as the first supporting version but does not document whether an older version rejects response_format or silently ignores it. Neither SDK does client-side gating, so this could not be settled without a live Azure resource. Never sending response_format below the floor is safe under either behavior: if the service would reject, no such call is made; if it would ignore, the native path is never taken so the prompt fallback still produces a parseable result. The rejected alternative was to send it and let the service decide, which is unsafe precisely in the silent-ignore case, since the caller would receive unconstrained text believing it was schema-constrained.

The comparison is a date-prefix comparison, not a validator. It assumes the documented zero-padded YYYY-MM-DD form, optionally suffixed -preview. Over that form, comparing the leading date lexicographically is exact. The GA v1 literal sorts above the floor, which matches Azure documenting v1 as supporting structured outputs. Values of other shapes are not classified reliably, and that is accepted rather than fixed: the service rejects an api-version it does not recognize, so the failure is immediate and loud, and duplicating Azure's version validation here would rot against it.

response_format was deliberately not added to the reserved-key set. Passing response_format without a schema is a working path today, and the reserved-key check would break it. That check also produces a message about reserved typed fields settable via the Setup, which does not describe this situation.

The collision check is scoped inside the branch that actually applied the schema. Hoisting it above the translation result would make it fire on a RowTypeInfo schema combined with a caller response_format, which is a working path.

Java needed a request-building seam before any of this was testable. The previous chat built the request inline with no way to observe it, and this module's test convention is a package-private seam with a real object rather than a mocking framework. buildRequest and toResponse exist for that reason. toResponse also has a functional consequence: splitting the method turned one read of model_of_azure_deployment into two independent reads that must agree, so it is read from the caller's map without consuming it.

Java derives the response format through a throwaway typed builder. StructuredOutputsKt.responseFormatFromClass is a Kotlin facade that is not callable from Java. toNativeResponseFormat therefore builds a minimal ChatCompletionCreateParams with the typed responseFormat(Class, JsonSchemaLocalValidation) overload, extracts the generated format from rawParams(), and reattaches it to the real builder. This produces the SDK's own strict draft-2020-12 schema rather than a hand-rolled one.

Python imports a private SDK helper. to_strict_json_schema comes from openai.lib._pydantic, which has a leading underscore. The openai client itself uses this helper to build the strict json_schema, and there is no public re-export. It has existed at that path since structured-output support landed in openai 1.66.3, which is the pinned minimum. A future openai bump that moves it fails loudly at import rather than silently degrading.

Java's JsonSchemaLocalValidation.NO is passed. Local validation is left to the provider rather than performed client-side.

4. Failure behavior

Missing deployment name. model absent or blank raises ValueError in Python and IllegalArgumentException in Java, before any request is issued. Unchanged from before.

Null or blank api-version, Java. The constructor raises IllegalArgumentException. The connection cannot be built, so no request path is reachable. Unchanged from before.

Reserved typed field inside additional_kwargs. Raises ValueError / IllegalArgumentException before the request. Unchanged from before.

Caller-supplied response_format colliding with an applied native schema. Raises ValueError / IllegalArgumentException before the request. The message names the schema and the deployment and states both remedies. This is new; before this change the combination was unreachable.

Unsupported configuration that is not an error. An unrecognized or absent backing model, an api-version below the floor, and a schema that does not translate all fall back silently to an unconstrained request. Nothing is logged and nothing raises. This is deliberate: the prompt-engineering fallback is expected to govern the response in those cases. It is also the most likely surprise for a user, since a capable deployment with model_of_azure_deployment unset takes this path.

An error returned by the service. In Java, doChat catches IllegalArgumentException and rethrows it unwrapped, and wraps every other exception in RuntimeException("Failed to call Azure OpenAI chat completions API.", e). This is unchanged from before, but note the seam moved: buildRequest is called inside the same try, so an argument error raised while building still propagates unwrapped. In Python there is no try/except in this path, so an SDK exception (including a 400 from the provider) propagates to the caller unchanged. That asymmetry predates this change and is not introduced by it.

A response that does not satisfy the requested schema. This connection does not validate the response against the schema. With strict: true, the provider is responsible for conformance, and a refusal or a length-truncated completion is returned as ordinary message content. The message is converted and returned to the caller unchanged. Parsing the content into the schema type happens above this layer, so a non-conforming body surfaces there as a parse failure, not here. There is no retry and no repair in this connection.

The SDK fails to produce a response format, Java. toNativeResponseFormat raises IllegalStateException naming the schema class if rawParams().responseFormat() is empty. This is a guard against an SDK behavior change, not an expected path.

Schema generation rejects the type. A BaseModel or POJO that the SDK's strict schema generator cannot express raises from the generator, before the request. Azure additionally enforces limits the generator does not check, for example nesting depth and open map fields becoming additionalProperties: true, which strict rejects. Those surface as a provider error on the first call. This behavior is inherited from the sibling OpenAI connection rather than introduced here.

Import-time failure, Python. If a future openai release moves openai.lib._pydantic.to_strict_json_schema, importing this module raises ImportError immediately rather than degrading at runtime.

5. Compatibility impact

No public type, field, or method signature changes. Java adds an override of a 4-arg chat that already exists on the base class, plus two package-private methods. Python's output_schema parameter already existed on this connection. No configuration field is added, removed, or made required.

No dependency changes. No pom.xml and no pyproject.toml edit. The Java path uses the openai-java SDK already present in this module.

A caller that passes no output schema sees identical behavior. The request is built by the same statements in the same order, and the response is converted identically.

One behavior does change for existing callers. Passing a non-null output_schema to this connection previously raised, since the connection had no native translation: NotImplementedError in Python via the base guard, and UnsupportedOperationException in Java via the base 4-arg default. It now either applies the native path or proceeds unconstrained. Nothing in the framework calls the 4-arg path today, so this is reachable only by a direct caller.

model_of_azure_deployment gains a second role. It previously labeled token-usage metrics only. It now also decides native capability. Its type, default, and absent-behavior are unchanged, and the Setup Javadoc was updated to say so. Callers who never set it are unaffected apart from staying on the fallback.

The cross-connection walk test changes population, not strictness. It exempts natively-translating connections. On this branch the exemption matches exactly one class and leaves the others held to the rejection contract. Its pre-existing non-empty assertion still runs on the filtered list, so an exemption that grew to cover everything would fail.

6. Tests to contracts

Python tests are in azure/tests/test_azure_openai_native_structured_output.py, a new non-integration file, because the existing Azure test module is integration-marked and would be skipped in the unit run. Java tests extend the existing AzureOpenAIChatModelConnectionTest; its five pre-existing tests are unmodified.

Test Language Contract pinned
test_native_applied_for_capable_deployment_model / testNativeAppliedForCapableDeploymentModel both C1
test_capable_native_request_still_targets_the_deployment / testCapableNativeRequestStillTargetsTheDeployment both C2
test_native_not_applied_when_deployment_model_absent / testNativeNotAppliedWhenDeploymentModelAbsent both C3
test_native_not_applied_for_unknown_deployment_model / testNativeNotAppliedForUnknownDeploymentModel both C3
test_native_not_applied_for_bare_gpt_4o / testNativeNotAppliedForBareGpt4o both C4
test_native_applied_for_other_api_versions_above_floor / testNativeAppliedForOtherApiVersionsAboveFloor (2024-10-21, v1) both C5, upper side
test_native_not_applied_when_api_version_below_floor / testNativeNotAppliedWhenApiVersionBelowFloor both C5
test_native_not_applied_when_api_version_empty Python only C5, empty case. Java has no equivalent because its constructor rejects blank, which the pre-existing testConstructorMissingApiVersion pins
test_native_not_applied_when_schema_none / testNativeNotAppliedWhenSchemaNull both C7
test_native_not_applied_for_row_type_info / testNativeNotAppliedForNonPojoSchema both C6
test_native_applied_even_when_tools_bound / testNativeAppliedEvenWhenToolsBound both C8
test_caller_response_format_conflicts_with_native_schema (parametrized over both channels) / testCallerResponseFormatConflictsWithNativeSchema both C9. Python covers two channels, Java one, matching the channel counts stated in C9
test_caller_response_format_survives_when_native_is_skipped (parametrized over 4 non-native paths x 2 channels) / testCallerResponseFormatSurvivesWhenNativeIsSkipped (nonNativePaths) both C10
test_capability_predicate_accepts_capable_models / testCapabilityPredicateAcceptsCapableModels both C1 capability half, and C14 by enumerating every allowlist entry
test_capability_predicate_rejects_incapable_models / testCapabilityPredicateRejectsIncapableModels both C3, C4
test_capability_predicate_reads_no_instance_state Python only C11
testResponseCarriesBackingModelTokenMetrics Java only C13, positive case
testResponseHandlingDoesNotConsumeCallerModelParams Java only C12
testNoTokenMetricsWhenMetricsInputsAreIncomplete (parametrized) Java only C13, negative cases: backing model unset, and usage absent
test_every_connection_rejects_an_output_schema_it_cannot_translate Python only C15

Contracts with no direct test, stated rather than omitted:

  • C11 has no Java equivalent. Java's capability predicate is protected and reads no instance state by inspection, but no Java test constructs an uninitialized instance to prove it. The Python test exists because Python's cross-connection walk actually calls the predicate on cls.__new__(cls).
  • C12 and C13 have no Python equivalent. Python's response handling was not restructured by this change, so its single read of model_of_azure_deployment is the pre-existing one and the two-reads-must-agree risk that motivates these tests does not exist there.
  • C14 is pinned only in the sense that both capability tests enumerate the same model list independently. No test compares the two languages' constants directly, since no cross-language test harness covers this pair.
  • The failure paths in section 4 that are inherited rather than introduced have no new tests: service errors, provider schema-limit rejections, and non-conforming responses. Existing coverage for the unchanged request and response paths applies.

@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 Jul 28, 2026
@weiqingy

Copy link
Copy Markdown
Collaborator Author

Revision 2. Section 4 previously said a refusal is returned as ordinary message content, which is wrong: a refusal arrives with empty content and is carried separately, and in Python it is dropped. Corrected below, with a new contract in section 2, a compatibility note in section 5, and the missing test coverage recorded in section 6. Revision 1 stays above unchanged.

Implementation Description

As-built description of the final code on this branch, derived by walking git diff origin/main...HEAD hunk by hunk.

1. Runtime flow

Both languages follow the same order. The connection decides everything locally; nothing upstream is consulted.

Python, AzureOpenAIChatModelConnection.chat:

  1. Tools are converted to OpenAI tool specs, unchanged from before.
  2. model is popped from kwargs as the Azure deployment name and required to be non-empty.
  3. model_of_azure_deployment is popped. This is the model backing the deployment.
  4. additional_kwargs is popped, and its keys are checked against the reserved typed fields, unchanged from before.
  5. The native gate is evaluated as three conditions in order: the schema is not None, supports_native_structured_output(model_of_azure_deployment) is true, and _api_version_supports_structured_output() is true.
  6. If the gate passes, _native_response_format(output_schema) is called. It returns None for anything that is not a BaseModel subclass, which is the fourth condition in practice.
  7. If it returned a format, the collision check runs: if response_format is present in kwargs or in additional_kwargs, ValueError is raised. Otherwise kwargs["response_format"] is set.
  8. client.chat.completions.create(...) is called with the deployment name as model. The response is converted by convert_from_openai_message at azure_openai_chat_model.py:319, unchanged from before.

Java, AzureOpenAIChatModelConnection:

  1. The 3-arg chat now delegates to doChat(..., null). The new 4-arg chat delegates to doChat(..., outputSchema).
  2. doChat calls buildRequest, issues client.chat().completions().create(params), and passes the completion to toResponse.
  3. buildRequest copies the caller's modelParams into a mutable map, then removes model (required non-blank) and model_of_azure_deployment from the copy.
  4. The native gate is evaluated as outputSchema instanceof Class, then supportsNativeStructuredOutput(modelOfAzureDeployment), then apiVersionSupportsStructuredOutput(). If all hold, toNativeResponseFormat(schemaClass) is attached to the builder and the schema's simple name is recorded in a local nativeSchemaName.
  5. temperature, max_tokens, and logprobs are read after that, unchanged from before.
  6. additional_kwargs is removed and checked for reserved typed fields, unchanged. Then, only if nativeSchemaName is non-null, the presence of response_format in additional_kwargs raises IllegalArgumentException. Otherwise every entry is written through as an additional body property, unchanged.
  7. toResponse converts the first choice's message through OpenAIChatCompletionsUtils.convertFromOpenAIMessage, then reads model_of_azure_deployment from the caller's original map with get rather than remove, and attaches model_name, promptTokens, and completionTokens only when that value is non-blank and completion.usage() is present.

The api-version value is read from a new private final String apiVersion field. Before this change the constructor consumed the value into AzureOpenAIServiceVersion.fromString(...) and discarded it.

Neither language inspects finish_reason at any point on this path.

2. Behavioral contracts

Each is a checkable claim about the final code.

C1. Native structured output is applied only when all four hold: the schema is non-null, the schema translates (a BaseModel subclass in Python, a Class in Java), the backing model is in the allowlist, and the api-version is at or above 2024-08-01. If any fails, the request is built exactly as it would have been without a schema.

C2. Capability is decided from model_of_azure_deployment, never from model. The request still targets model, the deployment name, as before.

C3. An absent, empty, or unrecognized model_of_azure_deployment reports not-capable, so the request carries no response_format.

C4. A bare gpt-4o reports not-capable, because Azure documents gpt-4o as supported only at versions 2024-08-06 and 2024-11-20 while 2024-05-13 is unsupported, and a model name carries no version.

C5. An api-version whose leading 10 characters sort below 2024-08-01 suppresses the native path. An empty api-version does the same in Python. In Java a null or blank api-version cannot reach the check, because the constructor already rejects it.

C6. A RowTypeInfo schema in Python, and any non-Class schema in Java, suppresses the native path and keeps the prompt-engineering fallback.

C7. A null schema leaves behavior identical to before this change.

C8. Bound tools do not suppress the native path. A request may carry both tools and response_format.

C9. When the native path applies and the caller also supplied response_format, the call raises rather than overwriting or duplicating it. Python checks both kwargs and additional_kwargs. Java checks additional_kwargs, which is its only caller-facing channel, because unrecognized top-level modelParams keys are discarded and never reach the request.

C10. On every path where the native format is not applied, a caller-supplied response_format reaches the provider untouched. In Python the identical object is forwarded, not a copy.

C11. The capability predicate reads no instance state. It is answerable on an instance that was never initialized, where field access would raise.

C12. Response handling does not consume the caller's modelParams. Java reads model_of_azure_deployment with get, so a caller reusing the same map across calls still sees it.

C13. Token metrics (model_name, promptTokens, completionTokens) are attached only when the backing model is non-blank and the completion carries usage. Neither alone is sufficient.

C14. The Java and Python allowlists and floor constant are identical, including ordering of the allowlist entries.

C15. A connection that translates schemas natively is exempt from the cross-connection walk that requires every connection to reject a schema it cannot translate. The exemption is derived from whether supports_native_structured_output is overridden, not from a name list.

C16 (new in this revision). A model refusal is not represented as message content, and the two languages differ in what survives it. The provider returns an empty content with the explanation in a separate refusal field. Java preserves it: OpenAIChatCompletionsUtils.convertFromOpenAIMessage sets content to message.content().orElse("") at line 114 and copies the refusal into extraArgs["refusal"] at line 117, so a caller sees an empty assistant message plus a recoverable refusal entry. Python discards it: convert_from_openai_message in openai_utils.py:217-222 sets content=message.content or "" and has no refusal handling, and the string refusal does not appear anywhere under python/flink_agents/, so nothing downstream recovers it. A Python caller receives an empty assistant message with no indication that a refusal occurred. This is a Java and Python divergence in observable behavior. It predates this change, as established in section 5.

3. Key decisions

Capability is keyed on model_of_azure_deployment, not on model. On Azure, model is the deployment name, an arbitrary string chosen by the user. Matching an allowlist against it is wrong in both directions: a deployment named prod-chat backed by a capable model would report not-capable, and a deployment named gpt-4o backed by anything would report capable. The rejected alternative was to reuse the OpenAI connection's rule unchanged, which is exactly that mistake. The cost of this choice is that leaving the optional model_of_azure_deployment unset keeps even a capable deployment on the fallback. That direction of error is the safe one.

Matching is exact, never by prefix. The sibling OpenAI connection matches one model family by prefix. Azure reports a model name and a model version as separate properties, so a name has no version suffix to discriminate on, and prefix matching would classify unsupported versions as capable.

The api-version floor is enforced in the request path, not in the capability predicate. The predicate's argument is a model name and carries no api-version, and the predicate must stay free of instance state (C11). Putting the floor check in the predicate would break that.

The floor is enforced conservatively because the failure mode below it is not documented. Azure documents 2024-08-01-preview as the first supporting version but does not document whether an older version rejects response_format or silently ignores it. Neither SDK does client-side gating, so this could not be settled without a live Azure resource. Never sending response_format below the floor is safe under either behavior: if the service would reject, no such call is made; if it would ignore, the native path is never taken so the prompt fallback still produces a parseable result. The rejected alternative was to send it and let the service decide, which is unsafe precisely in the silent-ignore case, since the caller would receive unconstrained text believing it was schema-constrained.

The comparison is a date-prefix comparison, not a validator. It assumes the documented zero-padded YYYY-MM-DD form, optionally suffixed -preview. Over that form, comparing the leading date lexicographically is exact. The GA v1 literal sorts above the floor, which matches Azure documenting v1 as supporting structured outputs. Values of other shapes are not classified reliably, and that is accepted rather than fixed: the service rejects an api-version it does not recognize, so the failure is immediate and loud, and duplicating Azure's version validation here would rot against it.

response_format was deliberately not added to the reserved-key set. Passing response_format without a schema is a working path today, and the reserved-key check would break it. That check also produces a message about reserved typed fields settable via the Setup, which does not describe this situation.

The collision check is scoped inside the branch that actually applied the schema. Hoisting it above the translation result would make it fire on a RowTypeInfo schema combined with a caller response_format, which is a working path.

Java needed a request-building seam before any of this was testable. The previous chat built the request inline with no way to observe it, and this module's test convention is a package-private seam with a real object rather than a mocking framework. buildRequest and toResponse exist for that reason. toResponse also has a functional consequence: splitting the method turned one read of model_of_azure_deployment into two independent reads that must agree, so it is read from the caller's map without consuming it.

Java derives the response format through a throwaway typed builder. StructuredOutputsKt.responseFormatFromClass is a Kotlin facade that is not callable from Java. toNativeResponseFormat therefore builds a minimal ChatCompletionCreateParams with the typed responseFormat(Class, JsonSchemaLocalValidation) overload, extracts the generated format from rawParams(), and reattaches it to the real builder. This produces the SDK's own strict draft-2020-12 schema rather than a hand-rolled one.

Python imports a private SDK helper. to_strict_json_schema comes from openai.lib._pydantic, which has a leading underscore. The openai client itself uses this helper to build the strict json_schema, and there is no public re-export. It has existed at that path since structured-output support landed in openai 1.66.3, which is the pinned minimum. A future openai bump that moves it fails loudly at import rather than silently degrading.

Java's JsonSchemaLocalValidation.NO is passed. Local validation is left to the provider rather than performed client-side.

Response handling was deliberately left unchanged. Both connections delegate to the shared conversion helper that already existed. That keeps this change scoped to the request side, and it is why the refusal representation described in C16 is inherited rather than addressed here.

4. Failure behavior

Missing deployment name. model absent or blank raises ValueError in Python and IllegalArgumentException in Java, before any request is issued. Unchanged from before.

Null or blank api-version, Java. The constructor raises IllegalArgumentException. The connection cannot be built, so no request path is reachable. Unchanged from before.

Reserved typed field inside additional_kwargs. Raises ValueError / IllegalArgumentException before the request. Unchanged from before.

Caller-supplied response_format colliding with an applied native schema. Raises ValueError / IllegalArgumentException before the request. The message names the schema and the deployment and states both remedies. This is new; before this change the combination was unreachable.

Unsupported configuration that is not an error. An unrecognized or absent backing model, an api-version below the floor, and a schema that does not translate all fall back silently to an unconstrained request. Nothing is logged and nothing raises. This is deliberate: the prompt-engineering fallback is expected to govern the response in those cases. It is also the most likely surprise for a user, since a capable deployment with model_of_azure_deployment unset takes this path.

An error returned by the service. In Java, doChat catches IllegalArgumentException and rethrows it unwrapped, and wraps every other exception in RuntimeException("Failed to call Azure OpenAI chat completions API.", e). This is unchanged from before, but note the seam moved: buildRequest is called inside the same try, so an argument error raised while building still propagates unwrapped. In Python there is no try/except in this path, so an SDK exception (including a 400 from the provider) propagates to the caller unchanged. That asymmetry predates this change and is not introduced by it.

A model refusal. Corrected in this revision. Under strict: true a model that will not answer refuses rather than emitting non-conforming JSON, so this is the primary non-happy path that native structured output introduces. The provider returns an empty content and puts the explanation in a separate refusal field. Neither connection raises, retries, or falls back. Java preserves the explanation in extraArgs["refusal"] (OpenAIChatCompletionsUtils.convertFromOpenAIMessage:114-117), so a caller can detect and read it, though only by inspecting extraArgs; an empty content on its own is indistinguishable from a genuinely empty completion. Python discards it (openai_utils.py:217-222, no refusal handling, and no occurrence of refusal anywhere under python/flink_agents/), so the refusal is silently absorbed and the caller receives an empty assistant message with the reason unavailable. Downstream parsing of that empty content into the schema type fails above this layer, with no indication that a refusal was the cause.

A length-truncated completion. finish_reason is not inspected in either language on this path. A truncated completion is returned as ordinary partial content, silently, and fails downstream at parse time. This part of the previous revision's statement was accurate; only the refusal half was wrong.

A response that otherwise does not satisfy the requested schema. This connection does not validate the response against the schema. With strict: true the provider is responsible for conformance. Parsing the content into the schema type happens above this layer, so a non-conforming body surfaces there as a parse failure, not here. There is no retry and no repair in this connection.

The SDK fails to produce a response format, Java. toNativeResponseFormat raises IllegalStateException naming the schema class if rawParams().responseFormat() is empty. This is a guard against an SDK behavior change, not an expected path.

Schema generation rejects the type. A BaseModel or POJO that the SDK's strict schema generator cannot express raises from the generator, before the request. Azure additionally enforces limits the generator does not check, for example nesting depth and open map fields becoming additionalProperties: true, which strict rejects. Those surface as a provider error on the first call. This behavior is inherited from the sibling OpenAI connection rather than introduced here.

Import-time failure, Python. If a future openai release moves openai.lib._pydantic.to_strict_json_schema, importing this module raises ImportError immediately rather than degrading at runtime.

5. Compatibility impact

No public type, field, or method signature changes. Java adds an override of a 4-arg chat that already exists on the base class, plus two package-private methods. Python's output_schema parameter already existed on this connection. No configuration field is added, removed, or made required.

No dependency changes. No pom.xml and no pyproject.toml edit. The Java path uses the openai-java SDK already present in this module.

A caller that passes no output schema sees identical behavior. The request is built by the same statements in the same order, and the response is converted identically.

One behavior does change for existing callers. Passing a non-null output_schema to this connection previously raised, since the connection had no native translation: NotImplementedError in Python via the base guard, and UnsupportedOperationException in Java via the base 4-arg default. It now either applies the native path or proceeds unconstrained. Nothing in the framework calls the 4-arg path today, so this is reachable only by a direct caller.

model_of_azure_deployment gains a second role. It previously labeled token-usage metrics only. It now also decides native capability. Its type, default, and absent-behavior are unchanged, and the Setup Javadoc was updated to say so. Callers who never set it are unaffected apart from staying on the fallback.

The refusal handling in C16 predates this change, but this change makes it easier to reach. openai_utils.py and OpenAIChatCompletionsUtils.java are not in this diff, and git log -S"refusal" -- python/flink_agents/ returns no commit, so the string has never existed on the Python side. A refusal was already possible for any Azure caller, since the provider populates that field for safety refusals generally. What changes is frequency: strict: true is the mode in which a model refuses instead of emitting non-conforming JSON, and before this change an Azure caller with a schema always took the prompt fallback and never sent strict: true. The gap is therefore inherited rather than introduced, and fixing it means changing shared response-conversion code used by the OpenAI connection as well, which is outside this change's request-side scope. It belongs in a separate change covering both connections.

The cross-connection walk test changes population, not strictness. It exempts natively-translating connections. On this branch the exemption matches exactly one class and leaves the others held to the rejection contract. Its pre-existing non-empty assertion still runs on the filtered list, so an exemption that grew to cover everything would fail.

6. Tests to contracts

Python tests are in azure/tests/test_azure_openai_native_structured_output.py, a new non-integration file, because the existing Azure test module is integration-marked and would be skipped in the unit run. Java tests extend the existing AzureOpenAIChatModelConnectionTest; its five pre-existing tests are unmodified.

Test Language Contract pinned
test_native_applied_for_capable_deployment_model / testNativeAppliedForCapableDeploymentModel both C1
test_capable_native_request_still_targets_the_deployment / testCapableNativeRequestStillTargetsTheDeployment both C2
test_native_not_applied_when_deployment_model_absent / testNativeNotAppliedWhenDeploymentModelAbsent both C3
test_native_not_applied_for_unknown_deployment_model / testNativeNotAppliedForUnknownDeploymentModel both C3
test_native_not_applied_for_bare_gpt_4o / testNativeNotAppliedForBareGpt4o both C4
test_native_applied_for_other_api_versions_above_floor / testNativeAppliedForOtherApiVersionsAboveFloor (2024-10-21, v1) both C5, upper side
test_native_not_applied_when_api_version_below_floor / testNativeNotAppliedWhenApiVersionBelowFloor both C5
test_native_not_applied_when_api_version_empty Python only C5, empty case. Java has no equivalent because its constructor rejects blank, which the pre-existing testConstructorMissingApiVersion pins
test_native_not_applied_when_schema_none / testNativeNotAppliedWhenSchemaNull both C7
test_native_not_applied_for_row_type_info / testNativeNotAppliedForNonPojoSchema both C6
test_native_applied_even_when_tools_bound / testNativeAppliedEvenWhenToolsBound both C8
test_caller_response_format_conflicts_with_native_schema (parametrized over both channels) / testCallerResponseFormatConflictsWithNativeSchema both C9. Python covers two channels, Java one, matching the channel counts stated in C9
test_caller_response_format_survives_when_native_is_skipped (parametrized over 4 non-native paths x 2 channels) / testCallerResponseFormatSurvivesWhenNativeIsSkipped (nonNativePaths) both C10
test_capability_predicate_accepts_capable_models / testCapabilityPredicateAcceptsCapableModels both C1 capability half, and C14 by enumerating every allowlist entry
test_capability_predicate_rejects_incapable_models / testCapabilityPredicateRejectsIncapableModels both C3, C4
test_capability_predicate_reads_no_instance_state Python only C11
testResponseCarriesBackingModelTokenMetrics Java only C13, positive case
testResponseHandlingDoesNotConsumeCallerModelParams Java only C12
testNoTokenMetricsWhenMetricsInputsAreIncomplete (parametrized) Java only C13, negative cases: backing model unset, and usage absent
test_every_connection_rejects_an_output_schema_it_cannot_translate Python only C15

Contracts with no direct test, stated rather than omitted:

  • C16 has no test in either language. Nothing constructs a completion carrying a refusal and asserts what the returned ChatMessage contains. This is the gap the correction in this revision exposes, and it is the least covered path of the ones this change makes reachable. The Java seam added here, toResponse, would make such a test straightforward, since a ChatCompletion with a refusal can be built offline from the SDK builders. Python has no equivalent seam today.
  • C11 has no Java equivalent. Java's capability predicate is protected and reads no instance state by inspection, but no Java test constructs an uninitialized instance to prove it. The Python test exists because Python's cross-connection walk actually calls the predicate on cls.__new__(cls).
  • C12 and C13 have no Python equivalent. Python's response handling was not restructured by this change, so its single read of model_of_azure_deployment is the pre-existing one and the two-reads-must-agree risk that motivates these tests does not exist there.
  • C14 is pinned only in the sense that both capability tests enumerate the same model list independently. No test compares the two languages' constants directly, since no cross-language test harness covers this pair.
  • The failure paths in section 4 that are inherited rather than introduced have no new tests: service errors, provider schema-limit rejections, truncated completions, and non-conforming responses. Existing coverage for the unchanged request and response paths applies.

@weiqingy

Copy link
Copy Markdown
Collaborator Author

@wenjin272 This is the PR I mentioned on #894 as the first sample for the Implementation Description experiment, so the description is up as a comment rather than in the body.

Current version is rev 2. Rev 1 is left above it untouched, since a reviewer approves a specific version and editing in place would change that silently.

Rev 2 exists because a check against the code found a mistake in rev 1. It stated that a provider refusal comes back as ordinary message content, and that is wrong: content is empty, Java preserves the reason in extraArgs["refusal"], and Python drops it entirely. That divergence, along with two related ones on the same path, is now #936. All three predate this PR, so the diff is unchanged, but native structured output is what makes the refusal path reachable for Azure callers, which is why the description now covers it.

Whenever you get to this one, the two-stage flow would be: read the description at the architecture and contract level first, then point a coding agent at the diff to check both that the code matches it and that it does not omit anything. I am curious whether your agent finds omissions that mine did not, since that check is the part carrying the most weight in the model.

@weiqingy

weiqingy commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

Spec

Problem

The Azure OpenAI connections reject a non-null output schema, so every caller with a schema stays on the prompt-engineering fallback even when the deployment's backing model supports native structured output. Azure OpenAI exposes the same response_format json-schema mechanism the OpenAI connection uses, so this is a gap in our support, not a provider limitation.

Proposed solution

Mirror the OpenAI native path in both Azure OpenAI connections: declare a capability predicate, translate a POJO/BaseModel schema into response_format, drop the rejection guard.

Two deviations from the OpenAI connection, both forced by the platform.

Capability resolves against model_of_azure_deployment, not model. On Azure model is the deployment name, an arbitrary string the user chose, so matching an allowlist against it is wrong in both directions.

A second conjunct gates on the configured api_version against Azure's documented 2024-08-01-preview floor. Whether an older version rejects response_format or silently ignores it could not be determined without a live Azure resource, and never sending it below the floor is safe under either behavior.

API surface

No new public type in either language. No config field added, removed, or made required. No signature change to an existing method.

Language Symbol Change
Python AzureOpenAIChatModelConnection.chat output_schema accepted instead of rejected
Python AzureOpenAIChatModelConnection.supports_native_structured_output override added, was inherited False
Java AzureOpenAIChatModelConnection.chat 4-arg overload override added, 3-arg behavior unchanged
Java AzureOpenAIChatModelConnection.supportsNativeStructuredOutput override added, was inherited false

Behavior

Input Result
capable model, POJO/BaseModel schema, api-version at or above the floor native response_format applied
RowTypeInfo schema not applied, prompt fallback
no output schema not applied, unchanged behavior for every existing caller
model_of_azure_deployment absent or not in the allowlist not applied, prompt fallback
model_of_azure_deployment is a bare gpt-4o not applied, the build version is unknowable on Azure
api-version below the floor or absent not applied, prompt fallback
every native condition met and the caller also supplied response_format raises, naming the schema and the conflicting parameter
caller supplies response_format on any non-native path passed through unchanged

Files expected to change

Two connection sources, two test files, plus the cross-connection walk test if the sibling OpenAI PR had not merged first.

Test coverage

Both languages pin the same set, in dedicated unit tests that need no Azure resource. Grouped by what each group proves rather than listed test by test.

What it pins Cases covered
Native output is applied capable backing model, POJO or BaseModel schema, api-version at or above the floor, including with tools bound; and the request still targets the deployment, not the backing model
Native output is withheld backing model absent, outside the allowlist, or a bare gpt-4o; api-version below the floor or empty; no schema; a RowTypeInfo schema
The api-version rule itself a later GA date and the GA v1 literal both clear the floor; a pre-floor version does not
The capability predicate every documented capable name is accepted; incapable, version-suffixed, and empty names are rejected
The collision guard fires a caller-supplied response_format meeting a native schema raises, on each channel that can carry it
The collision guard does not over-fire a caller-supplied response_format passes through untouched on every non-native path

Java also pins that token metrics still label the response with the backing model, and that response handling leaves the caller's model params intact. Only Java needed the request/response split, so only Java can regress there.

Python also pins that the capability predicate answers on an uninitialized instance. That is what lets the cross-connection walk test call it without credentials.

Not tested on purpose: anything needing a live Azure resource, and the behavior of a pre-floor api-version, which the design never sends.

Out of scope

Azure AI Inference native output, verified impossible at the pinned SDK and recorded on #280. Ollama, whose capability is server-version-gated rather than model-gated and needs its own design. Threading a schema down from an action or an agent, which the foundation deliberately left out. RowTypeInfo native support. Pre-validating a schema against Azure's structured-output limits, which fail loudly at the provider and are inherited from the OpenAI connection rather than introduced here.

@weiqingy

Copy link
Copy Markdown
Collaborator Author

@wenjin272 Following up on the ping above.

A Spec is now posted as a comment too. The Implementation Description is as-built, written from the finished code. The Spec is what the change was meant to do, written before the code, and is much shorter.

Could you please take a look at the PR and let me know which one is better from your review point of view?

If you only have time to check one spot, please look at toNativeResponseFormat in AzureOpenAIChatModelConnection.java. It builds a throwaway params object to reach the SDK's schema builder, and has an IllegalStateException that no test exercises.

@wenjin272 wenjin272 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I found three issues that appear to conflict with the proposed behavior.

// applies only for a POJO Class schema — a RowTypeInfo (wrapped in OutputSchema) keeps the
// prompt-engineering fallback, as do an incapable model and an api-version below the floor.
String nativeSchemaName = null;
if (outputSchema instanceof Class

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On every non-native branch, this omits response_format and still sends the request. The connection neither adds a schema prompt nor signals the caller to fall back, so a non-null outputSchema is silently dropped and direct callers receive an unconstrained response instead of the previous exception. Could we reject the schema when native format cannot be applied, leaving the caller to invoke the prompt path with a null schema?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

You're right, the exception is gone. Azure previously inherited the base 4-arg chat, which throws on a non-null schema, and Python called _reject_unsupported_output_schema. This PR removes both.

This is the same drop as #919 (r3671993321), where you agreed the fix belongs in #912 and asked for a TODO at the capability check. That marker shipped in e7639ed, but this branch predates it. Added in 4d60405 for both languages, including the two Azure-only cases: an api-version below the floor, and model_of_azure_deployment unset.

I'd rather not make Azure reject on its own, though. The OpenAI connection is merged with the opposite behavior and tests that assert it, so throwing only here would leave the two disagreeing. Happy to pull the change forward if you prefer, as long as it lands in both connections together.

"""
if not self.api_version:
return False
return self.api_version[:10] >= _MIN_STRUCTURED_OUTPUT_API_VERSION

@wenjin272 wenjin272 Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

v1 passes this gate only because the comparison is lexicographic: "v1" >= "2024-08-01" evaluates to true. However, the current AzureOpenAI client still sends the legacy /openai/deployments/{model}/chat/completions?api-version=v1 request, while Azure v1 uses /openai/v1/chat/completions. Since the mocked test does not inspect the final URL, could we remove v1 from this gate/test until both connections support the unified endpoint?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Agreed, v1 was passing on string ordering alone.

Fixed in 62c6a1d. The gate now classifies only the dated form Azure documents, so v1 and any other non-dated value report not-capable and keep the prompt fallback. I changed the gate rather than just the test, since otherwise the code would still admit v1. Both suites assert that now, with latest as a second case. The documented examples use 2024-10-21, so nothing regresses.

One asymmetry worth flagging: in openai-java, azure_url_path_mode=UNIFIED or an endpoint ending in /openai/v1 does reach the unified endpoint, so v1 is reachable there in a way it is not from Python's AzureOpenAI. Not-capable is still the right answer, it just costs a prompt fallback rather than being impossible. The Java javadoc says so; the Python docstring does not, since there is no equivalent option.

# here because Azure model identifiers do not contain spaces.
_NATIVE_STRUCTURED_OUTPUT_MODELS = frozenset(
{
"gpt-5.1-codex",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This allowlist includes Responses-only models such as gpt-5.1-codex, gpt-5.1-codex-mini, and gpt-5-codex, but this connection calls chat.completions.create. Could we intersect the Structured Outputs list with the models that Azure supports on the Chat Completions API?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, and it was broader than the three you named.

Fixed in 62c6a1d. The set is now the intersection of the structured-outputs list with the models Azure serves on Chat Completions. The "Chat Completions API" row on the reasoning page also marks gpt-5-pro, codex-mini and o3-pro unsupported, so six come out and thirteen remain.

Both rejection tests now pin all six by name, so re-adding one gets caught.

@wenjin272

Copy link
Copy Markdown
Contributor

@weiqingy Here is my feedback after using #930 as our first experiment with developer-side artifacts for AI-assisted review.

For this PR, I found the Spec more useful than the Implementation Description. The change is focused, the production-code diff is relatively small, and its impact is limited to a specific integration rather than a critical runtime path. The Spec gave me enough context to understand the design intent, behavioral boundaries, and key implementation decisions. By comparison, the Implementation Description sometimes went into so much detail that reading the code directly was more efficient.

This is partly contextual. I am already familiar with and confident in Weiqing’s code quality, so I did not feel the need to inspect every implementation detail manually. A detailed as-built description may be more valuable for a larger, cross-cutting, unfamiliar, or critical-path change. For a PR of this size and risk profile, however, a concise Spec seems to be the better reviewer artifact.

As a second-stage experiment, I also asked Codex to review the implementation against the Spec, generate findings, and post inline comments directly to GitHub. This workflow was practical: the Spec served as the review anchor, while the coding agent handled the detailed inspection of the implementation.

This is only one data point, but my current impression is that the appropriate level of description should depend on the PR’s size and risk. For focused changes like this one, a Spec may provide most of the review benefit with less authoring and reading overhead.

…t capability gate

Two conditions decided native structured output too generously.

The model allowlist was taken from Azure's structured-outputs page, which lists
the models supporting the feature on any API. This connection calls the Chat
Completions API, so the set has to be the intersection with the models Azure
serves there. Six entries were served only on the Responses API and are removed:
gpt-5.1-codex, gpt-5.1-codex-mini, gpt-5-pro, gpt-5-codex, codex-mini and o3-pro.
Thirteen entries remain. Both rejection tests now pin all six so re-adding one is
caught.

The api-version gate compared the leading characters lexicographically against
the floor, which admitted the GA "v1" literal as a string-ordering artifact. The
gate now classifies only the api-version form Azure documents, a zero-padded
YYYY-MM-DD date; any other value reports not-capable and keeps the
prompt-engineering fallback. No dated api-version in the pinned SDK is newly
rejected, and the documented example value is unaffected.

Java and Python keep identical model sets and gate semantics.
…he Azure connection

The connection never sees the requested structured-output strategy, so its
capability re-check cannot tell an explicit NATIVE request apart from an AUTO
that merely resolved to native. Whenever the native branch is skipped the caller
gets an unconstrained response rather than an error. On Azure that also happens
below the api-version floor and when model_of_azure_deployment is unset, so
capability cannot be resolved at all.

Records this at both capability re-check sites, matching the marker the OpenAI
connection already carries.
@weiqingy

weiqingy commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

@wenjin272 Thanks for the review. I have updated the PR to address the three comments, could you take another look?

On the experiment feedback, this is useful from a single PR.

I agree it was too detailed, though I suspect the cause was my prompt rather than the format itself. I had told the agent to make the description self-contained so a reviewing agent would only need one document. That optimized for machines and made it unreadable for humans: 24k characters for a 1,232-line diff.

That rule is gone now. The description is the PR body, and #952 is the corrected sample: 123 lines, around 6k characters. Curious whether it reads better to you.

I also agree on scaling detail to size and risk. Maybe a rule along these lines:

  • Keep failure behavior and the contracts-to-tests mapping at any size, since those are properties of the finished code that a design-intent Spec tends to leave out.
  • Add the runtime-flow narration only for large or cross-cutting changes, since on a focused PR it mostly restates the diff.

Does that split match what you had in mind, or would you draw the line differently?

@wenjin272

Copy link
Copy Markdown
Contributor

Thanks @weiqingy, this is broadly aligned with what I had in mind.

What reviewers need is a clear explanation of the PR’s design intent and key decisions. Whether that comes from a developer-written Spec or an as-built description derived from the code is less important. What we should standardize is the content that this reviewer-facing description is expected to cover.

I agree that failure behavior and test coverage should be included. For large or cross-cutting PRs, the description may also need diagrams or a concise explanation of the runtime flow to help reviewers understand the change without reconstructing everything from the diff.

I have not yet formed a complete view of what the required structure should be. At this point, I think the minimum should include:

  • Design intent
  • Key decisions and trade-offs
  • Failure behavior
  • Test coverage

We can refine this list as we gather more experience from additional PRs.

@wenjin272
wenjin272 merged commit 5cc3b58 into apache:main Aug 4, 2026
27 checks passed
@weiqingy

weiqingy commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

@wenjin272 Thanks for the review.

Agreed, and agreed that what the description covers matters more than where it came from.

Your four look right. I would add two more. Both came up on #952, which is only 123 lines, so this is not just a big-PR thing.

Compatibility impact. Writing it made me look at a file the diff never touches, and I found that a refusal we now store on a message gets sent back to the provider on the next request. Nothing breaks, and Java does the same, but it is a real behavior change that none of the design docs mentioned. The #930 miss that became #936 was the same kind of thing, in a file outside the diff.

Behavioral contracts. Listing what the code promises, instead of listing the tests, is what showed me one promise had no test at all. I added it before opening the PR. A list of tests would have looked fine.

So six: design intent, key decisions, contracts, failure behavior, a test for each contract, compatibility. Runtime flow and diagrams only for big PRs, as you say.

Starting with your four and adding as we go is fine too. #965 is 919 lines, so it should tell us more about the big end.

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

Labels

doc-not-needed Your PR changes do not impact docs fixVersion/0.4.0 priority/major Default priority of the PR or issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants