Skip to content

[Needs design] Update openai provider types - #431

Draft
github-actions[bot] wants to merge 1 commit into
mainfrom
update-openai-provider-types-b5059f39-32049158364
Draft

[Needs design] Update openai provider types#431
github-actions[bot] wants to merge 1 commit into
mainfrom
update-openai-provider-types-b5059f39-32049158364

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Automated update of Lingua provider types.

Provider: openai

Publication mode: needs-design draft

Feedback: comment /bt good or /bt bad to log review feedback to the Braintrust trace.

Human decisions required

The agent completed every unblocked item. These decisions require human input before the update can be finished:

  • FunctionToolCallOutput.namespace / FunctionCallOutputItemParam.namespace (+ Beta twins) (function-call-output-namespace)

    • Question: What is the canonical universal representation for a tool-RESULT namespace on OpenAI Responses function_call_output and custom_tool_call_output items, given that ToolResultContentPart has no namespace field and the only existing precedent (tool-CALL namespace) rides in the opaque provider_options carrier that the typed-boundary rule forbids extending? Secondarily: is the output-side namespace authoritative, or is it always an echo of the originating call's namespace? And should the new request-side constraints (name 1..128; namespace 1..64 matching ^[a-zA-Z0-9_-]+$) be validated at the Lingua boundary or passed through for OpenAI to reject?
    • Evidence: openapi.yml:58275-58278 adds namespace to the response schema and openapi.yml:87008-87013 adds it to the request schema with maxLength 64, minLength 1, and pattern ^[a-zA-Z0-9_-]+$; Beta mirrors at :97348-97353 and :94739-94746. Generated types absorbed this as doc comments only (generated.rs:6035-6037), because OutputItem.namespace (:6040) and InputItem.namespace (:2418) already existed via union flattening, so there is no compile-time signal. The value is dropped in all four directions: import hardcodes provider_options: None at convert.rs:1676 while the tool-CALL arm one branch above at convert.rs:1639 calls provider_options_from_openai_tool_call(input.namespace); the two request exports at convert.rs:2624-2637 and :2899-2910 and the response export at convert.rs:3858-3868 all end in ..Default::default() and never set namespace, whereas the tool-call exports set it at convert.rs:2803, :2831, :2845, :2971, :4268, :4287, and :4302. ToolResultContentPart (universal/message.rs:219-227) has no namespace field, and grep over crates/lingua/src/universal/ finds namespace only as a tool-DEFINITION discriminator at tools.rs:142 and :1093-1097. No other provider has the concept; the nearest neighbor, responses_namespace at providers/anthropic/tool_discovery.rs:355, is a tool-definition namespace. The loss is reproducible today in payloads/snapshots/responsesToolSearchInputParam/responses/followup-request.json, where the function_call at lines 46-53 carries "namespace": "search_code" and the paired function_call_output at lines 55-59 carries neither name nor namespace. Critically, the expected-difference entry at crates/coverage-report/src/requests_expected_differences.json:481 exempts messages[].content[].provider_options for this exact test case with target "*", so a provider_options-based fix would be masked even for Responses->Responses and would appear to pass without working.
    • Recommended option: Add a first-class typed field to the universal tool-result type and, in the same change, promote the tool-CALL namespace off provider_options so both halves of a tool interaction share one canonical representation. Concretely: add namespace: Option to ToolResultContentPart (universal/message.rs:219-227) and to AssistantContentPart::ToolCall, then retire provider_options_from_openai_tool_call and openai_tool_call_provider_options_view (convert.rs:818-840) in favor of direct field mapping. This satisfies the typed-boundary rule, removes an existing opaque carrier rather than adding one, makes the loss visible to the coverage suite, and lets the entry at requests_expected_differences.json:481 be narrowed to the Anthropic, ChatCompletions, and Bedrock targets its stated reason actually describes. Treat the output-side namespace as authoritative rather than derived, because the request-side field carries its own independent 1..64 pattern constraint, which implies clients may set it without reference to the call. On the constraint sub-question, recommend passing through without boundary validation, consistent with the absence of any length or pattern validation elsewhere in convert.rs, whose only validators are validate_tool_caller (:45) and validate_openai_prompt_cache_ttl (:369).
    • Alternatives:
      • Introduce a shared qualified tool-identity type (namespace plus name) used by both AssistantContentPart::ToolCall and ToolResultContentPart. Tradeoff: the cleanest long-term model and it makes call/result pairing structural, but it is a wider breaking change to the universal schema and its TS bindings, touching every provider's tool converters rather than only OpenAI's, so it carries the largest blast radius for a two-field spec addition.
      • Add namespace: Option to ToolResultContentPart only, leaving the tool-CALL namespace in provider_options. Tradeoff: smallest diff and unblocks the spec change immediately, but leaves two different representations for the same concept on the two halves of one tool interaction, which is the asymmetry that caused this bug, and it keeps the masking entry at requests_expected_differences.json:481 load-bearing for the call side.
      • Derive the result namespace from the matched tool_call_id and never store it, relying on the originating call's namespace. Tradeoff: zero universal schema change, but it is only sound if OpenAI guarantees the two always agree. The spec does not state that, and the independent minLength/maxLength/pattern constraints on the request-side field suggest clients may set it independently, so this risks silently rewriting a caller-supplied value.
      • Carry it in provider_options mirroring convert.rs:829-840. Tradeoff: smallest possible change and consistent with existing code, but explicitly rejected here: it is an opaque round-trip-only channel that violates the typed-boundary rule, and it lands inside the target "*" exemption at requests_expected_differences.json:481, so the coverage suite could not tell a working fix from a broken one.
    • Likely files:
      • crates/lingua/src/universal/message.rs
      • crates/lingua/src/providers/openai/convert.rs
      • crates/lingua/src/providers/openai/responses_adapter.rs
      • crates/coverage-report/src/requests_expected_differences.json
      • payloads/cases/params.ts
      • payloads/snapshots/responsesToolSearchInputParam/responses/followup-request.json
    • Validation commands:
      • cargo test -p lingua --lib providers::openai::convert
      • cargo test -p lingua --lib providers::openai::responses_adapter
      • cargo test -p lingua --lib universal::
      • make test-payloads
      • cargo test -p coverage-report --test cross_provider_test cross_provider_transformations_have_no_unexpected_failures
      • make typed-boundary-check
  • FunctionToolCallOutput.name / FunctionCallOutputItemParam.name (+ Beta twins) (function-call-output-name)

    • Question: Does the OpenAI Responses API emit function_call_output and custom_tool_call_output items in the streaming event sequence (response.output_item.added / response.output_item.done), now that the response schema at openapi.yml:58271-58278 defines name and namespace on them? If so, how should the streaming adapter surface an output item that is not a tool-call start, given that the terminal response.completed event carries no output array to reconstruct from?
    • Evidence: openapi.yml:58271-58278 adds name and namespace to FunctionToolCallOutput, which reaches Response via FunctionToolCallOutputResource (:58311-58313) and OutputItem (:62854). That OpenAI added these to the RESPONSE schema is the signal that such items now appear in output arrays. The non-streaming path drops them entirely: there is no OutputItemType::FunctionCallOutput import arm in convert.rs (grep confirms OutputItemType::FunctionCallOutput appears only at :3853 and :3855, both in the export match), so they fall to '_ => continue' at convert.rs:3802-3805, and the OutputItem->InputItem bridge yields input_item_type: None via '_ => None' at convert.rs:3157-3201. On the streaming path, ResponsesOutputItemAddedItem (responses_adapter.rs:344-362) models only function_call and custom_tool_call and routes everything else to '#[serde(other)] Other' at :359-361. That is correct for its documented purpose, tool_call_start tracking at :364-370, but there is no fallback: the response.completed handler at responses_adapter.rs:1483-1512 reads only usage, model, id, and served_service_tier and never re-imports response.output. So a streamed function_call_output would be lost with no recovery path, and no offline fixture can establish whether OpenAI emits one.
    • Recommended option: Fix the non-streaming response-import gap now, since it is unambiguous and has an exact universal home in ToolResultContentPart.tool_name (universal/message.rs:221): add an OutputItemType::FunctionCallOutput | CustomToolCallOutput arm to the OutputItem->Message import and a corresponding arm to the OutputItem->InputItem type map at convert.rs:3157-3201. Then run a live capture of responsesToolSearchInputParam against the Responses target with streaming enabled and inspect the raw event sequence for function_call_output items. If the capture shows none, record streaming as not_affected with that capture as evidence and leave ResponsesOutputItemAddedItem unchanged, since its Other arm is correct by design. If the capture shows them, extend ResponsesOutputItemAddedItem with an explicit variant and route it through the same import path rather than adding a second parallel assembly mechanism.
    • Alternatives:
      • Add a function_call_output variant to ResponsesOutputItemAddedItem pre-emptively without a capture. Tradeoff: defensive and cheap, but it adds an untested code path for an event that may never be emitted, and an untested branch that looks handled is worse than an honest gap because it removes the pressure to verify.
      • Re-import response.output from the terminal response.completed payload as a general safety net for any output item the delta machinery does not model. Tradeoff: closes this and every future output-item gap at once, but it risks double-emitting content already assembled from deltas and is a significant change to streaming assembly semantics for a two-field spec addition.
      • Treat streaming as not_affected on the reasoning that these are client-supplied input items merely echoed back, and that the delta machinery is scoped to tool-call starts by design. Tradeoff: likely correct and requires no work, but it is an assumption about server behavior contradicted by OpenAI having just added these fields to the response schema, and the skill forbids using not_affected without concrete evidence.
    • Likely files:
      • crates/lingua/src/providers/openai/convert.rs
      • crates/lingua/src/providers/openai/responses_adapter.rs
      • payloads/cases/params.ts
      • payloads/snapshots/responsesToolSearchInputParam/responses/followup-request.json
    • Validation commands:
      • cargo test -p lingua --lib providers::openai::convert
      • cargo test -p lingua --lib providers::openai::responses_adapter
      • cargo test -p lingua --test import_fixtures
      • make test-payloads
      • cargo test -p coverage-report --test cross_provider_test cross_provider_transformations_have_no_unexpected_failures
  • ModelIdsShared / BetaModelIdsShared enum member 'gpt-5.5' (model-ids-gpt-5-5)

    • Question: Which EffortFamily does gpt-5.5 belong to, and specifically does it support the 'max' reasoning effort? The neighbouring releases bracket but do not determine it: gpt-5.4 maps to NoneLowMediumHighXhigh and gpt-5.6 to NoneLowMediumHighXhighMax. A related decision: should the identical pre-existing fall-through for gpt-5.3 non-codex be closed in the same edit, and should reasoning_effort_family_for_model be restructured to parse the point release numerically (as supports_prompt_cache_breakpoint already does) so that future point releases fail loudly instead of silently returning None?
    • Evidence: gpt_5_point_release_suffix (capabilities.rs:168-172) returns Some("5") for gpt-5.5. That value matches no branch in reasoning_effort_family_for_model: not :178 ('6'), :180-181 ('4' or '2'), :188 ('3-codex'), :190 ('1-codex'), or :192 ('1'). It then fails the fallback at :194-197 because "gpt-5.5".starts_with("gpt-5-") is false, the separator being a dot rather than a dash, and returns None at :201. clamp_reasoning_effort_for_model then early-returns the effort unmodified at capabilities.rs:210-212, so Minimal and Max reach the provider verbatim on the cross-provider emission paths at adapter.rs:552 and responses_adapter.rs:1045. The specification cannot settle this: ReasoningEffort at openapi.yml:74901-74918 is a single global enum listing all seven values with only the caveat that not all reasoning models support every one, and there is no per-model effort table. The existing table test at capabilities.rs:489-540 covers 5.1, 5.1-codex, 5.2, 5.2-codex, 5.3-codex, 5.4, 5.6-terra, gpt-5, gpt-5-mini, gpt-5-nano, gpt-4o, and two databricks- forms, with no 5.5 row and no fall-through assertion, which is why this shipped undetected. The gap is live rather than hypothetical: payloads/cases/params.ts:253 already pins model: "gpt-5.5". Contrast supports_prompt_cache_breakpoint (capabilities.rs:78-95), which parses major and minor numerically and therefore handles gpt-5.5 correctly, returning false for (5,5) as already asserted at capabilities.rs:435. One piece of countervailing context worth checking before trusting that: openapi.yml:62223 states that for gpt-5.5, gpt-5.5-pro, and future models only 24h prompt_cache_retention is supported, which groups gpt-5.5 with newer models and sits oddly against a >= (5,6) breakpoint gate. That line is pre-existing and not part of this diff, so it is context rather than a required change.
    • Recommended option: Confirm the family from the gpt-5.5 model card or a targeted probe rather than inferring it from the version ordering, since the enum insertion position between gpt-5.6-luna and gpt-5.4 reflects numeric sort order and carries no capability information. If forced to choose before confirmation, assign NoneLowMediumHighXhigh, matching gpt-5.4, on the grounds that 'max' was introduced with the 5.6 family and clamping down is the safe direction: an over-clamp degrades a request that would have succeeded, while an under-clamp sends a value the provider rejects outright. Alongside whichever value is chosen, restructure the point-release dispatch to parse numerically like supports_prompt_cache_breakpoint at capabilities.rs:78-95, and add a fall-through assertion to the test table at :489-540, so the next point release cannot silently reach the None branch. Close the gpt-5.3 non-codex hole in the same edit.
    • Alternatives:
      • Assign NoneLowMediumHighXhighMax, matching gpt-5.6. Tradeoff: correct if gpt-5.5 does support max, and avoids needlessly degrading requests, but if wrong it lets an unsupported effort reach the provider and produces a request-time API error rather than a graceful clamp, which is the exact failure clamp_reasoning_effort_for_model exists to prevent.
      • Leave the None fall-through in place and treat unclamped pass-through as deliberate fail-open behavior. Tradeoff: zero risk of over-clamping and no guess required, but it is inconsistent with every other supported gpt-5 point release and makes the clamp silently inapplicable to a model the payload suite already exercises at payloads/cases/params.ts:253.
      • Make an unrecognized gpt-5 point release a hard error instead of returning None. Tradeoff: guarantees no future model is silently unclamped, which is the root cause here, but it converts a graceful degradation into a runtime failure for models that are valid but simply newer than the table, and would need a rollout plan of its own.
    • Likely files:
      • crates/lingua/src/providers/openai/capabilities.rs
      • crates/lingua/src/providers/openai/adapter.rs
      • crates/lingua/src/providers/openai/responses_adapter.rs
      • crates/braintrust-llm-router/src/catalog/spec.rs
    • Validation commands:
      • cargo test -p lingua --lib providers::openai::capabilities::tests::test_clamp_reasoning_effort_for_model
      • cargo test -p lingua --lib providers::openai::capabilities
      • cargo test -p braintrust-llm-router --lib catalog::spec
      • make test-payloads

Validation

  • ./pipelines/generate-provider-types.sh openai: success
  • make generate-types PROVIDER=openai: success
  • Braintrust workflow trace: success
  • Claude integration plan: success
  • Structured plan validation: success
  • Human design blockers: true
  • Claude focused implementation: success
  • Immutable plan revalidation: success
  • Initial post-implementation Rust regeneration: success
  • Initial provider update path policy: success
  • Initial provider semantic policy: success
  • Initial post-implementation TypeScript regeneration: success
  • Initial formatting: success
  • Initial focused provider tests: success
  • Initial conditional generator tests: success
  • Initial build: success
  • Initial clippy: success
  • Bounded Claude mechanical repair: skipped
  • Effective mechanical validation source: initial
  • Effective mechanical validation: success
  • make lingua-wasm: success
  • Planned live capture cases: ``
  • Live capture (OpenAI): skipped
  • Live capture (Anthropic): skipped
  • Live capture (Google): skipped
  • Planned cross-provider transform capture: skipped
  • Payload fixture sync: success
  • make test-payloads: success
  • make typed-boundary-check: success
  • cargo test -p coverage-report --test cross_provider_test cross_provider_transformations_have_no_unexpected_failures: success
  • Claude read-only verification: success
  • Verification report validation: success
  • Verification verdict: fail
  • Recoverable binary patch archive: success

Ready PRs have no blockers and a passing verification verdict. Draft PRs may contain explicit human design blockers, but every automated safety and deterministic validation still passed. Failed runs retain the exact binary patch in the workflow artifact for manual recovery.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant