fix: drop the redundant stream_options patch that OpenAI 2.13.0 rejects - #495
Conversation
orbit-api main was red. Nine AiIntentServiceStreamingTests failed after #479 bumped OpenAI 2.12.0 to 2.13.0, and the failure was invisible on main because API Tests only ran on pull_request. Root cause, traced rather than inferred. The streamed round called options.Patch.Set("$.stream_options.include_usage"u8, true) through the experimental SCME0001 JsonPatch API. In 2.13.0 ChatCompletionOptions gained a PropagateSet override for that path which dereferences a null internal StreamOptions, so it throws NullReferenceException. Measured: an arbitrary nested patch such as $.a.b still succeeds, and only $.stream_options.* throws, so this is specific to that path and not a general JsonPatch break. AiIntentService swallowed the NRE into Result.Failure(AiUnavailable), which is why the assertion only ever reported IsSuccess false. The patch was never needed. The SDK already emits "stream_options":{"include_usage":true} for a streamed round on its own, verified by dumping the outgoing request body with the call removed, on both 2.12.0 and 2.13.0. So this deletes the call and its helper instead of pinning the package backwards, which keeps us on the current SDK and drops an experimental-API suppression. A test now asserts the request carries stream_options.include_usage. We no longer set it ourselves, so if a future SDK stops emitting it, per-user token accounting would silently record nothing. This is the guard for that. API Tests now also runs on push to main. Every other gate here already did, which is why a red main showed up only as one failing SonarCloud Analysis. Full suite: 547 domain, 3208 application, 2169 infrastructure, 32 analyzer, all passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Important
The new push trigger reaches pull request only jobs that use an unset github.base_ref, so API Tests will fail on every main push.
Reviewed changes I reviewed the OpenAI streaming regression fix, its request-shape coverage, and the expanded API test workflow trigger.
- Streaming patch removal: Removes the redundant experimental
JsonPatchcall that OpenAI 2.13.0 rejects while retaining the SDK's emitted usage option. - Usage regression coverage: Adds an exact outbound JSON assertion for
stream_options.include_usageso usage accounting cannot silently lose its SDK prerequisite. - Main branch test coverage: Adds
pushcoverage formain, but currently exposes pull request only assumptions in two workflow jobs.
GPT Sol | 𝕏
…push Adding the push trigger reached two jobs that diff HEAD against github.base_ref, which GitHub leaves empty on a push event. Guard Migrations and the OpenAPI Breaking-Change Gate would both have failed on every push to main, turning the new gate into permanent noise. Both are inherently base-relative, so they are scoped to pull_request rather than taught a fallback base. Build, Unit Tests, Dependency Scan and Guard Conventions read only the working tree and stay on both events, which is the coverage the push trigger was added for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes I reviewed the workflow guards added since the prior Pullfrog review and checked them against the complete PR diff and the API Tests job dependency graph.
- Push safe workflow: Scoped
migration-guardandopenapi-breakingto pull request events, preserving theirgithub.base_refcomparisons while leaving build and unit tests active on pushes tomain.
GPT Sol | 𝕏
|




Ticket: #364 in the ticket repository, https://github.com/thomasluizon/orbit-tickets/issues/364
mainis red and this unblocks it. Every open pull request against this repository inherits the failure, including #494.What broke
#479, the Dependabot
nuget-minor-patchgroup, bumpedOpenAI2.12.0 to 2.13.0. NineAiIntentServiceStreamingTestswent red.It was invisible on
mainbecauseAPI Testsonly ran onpull_request, so a redmainsurfaced as a single failingSonarCloud Analysis, whose "Test with Coverage" job is what actually runs the suite.Root cause, traced rather than inferred
The streamed round asked for usage through the experimental
SCME0001JsonPatch API:In 2.13.0,
ChatCompletionOptionsgained aPropagateSetoverride for that path which dereferences a null internalStreamOptions, so it throwsNullReferenceException. Measured directly against the installed 2.13.0 assembly:$.foo(top-level bool)$.a.b(arbitrary nested)$.stream_options.include_usageSo this is specific to
stream_options, not a generalJsonPatchbreak.ChatCompletionOptions.StreamOptionsisinternalin 2.13.0 and there is no public typed replacement, soPatchwas the only documented route.AiIntentServiceswallowed the NRE intoResult.Failure(AiUnavailable), which is why every assertion only ever reportedIsSuccessfalse with no cause. I surfaced it by temporarily rethrowing.Why this deletes the call instead of pinning the package
The patch was never needed. The SDK already emits
"stream_options":{"include_usage":true}for a streamed round on its own. Verified by dumping the outgoing request body with the call removed:That holds on both 2.12.0 and 2.13.0.
So the options were: pin to 2.12.0 (2.13.0 is the latest on nuget.org, so there is no forward fix to take instead), or delete code that does nothing. Deleting is strictly better: we stay on the current SDK, we take no Dependabot ignore, and an experimental-API suppression leaves the codebase.
The new test is the guard for that reasoning
We no longer request usage ourselves, so we now depend on SDK behaviour.
SendWithToolsAsync_Streaming_RequestsUsageInStreamOptionsasserts the outgoing request carriesstream_options.include_usage. If a future SDK stops emitting it, per-user token accounting would silently record nothing, and ORB-100's per-request cost bound would quietly stop working. This test fails instead.API Testsnow runs on push tomainarch-map.yml,guards.ymlandcodeql.ymlalready do.test.ymldid not, which is the reason a redmainwas only visible as one confusingly-named failing check.Verification
Full suite, locally:
Orbit.Domain.TestsOrbit.Application.TestsOrbit.Infrastructure.TestsOrbit.Analyzers.TestsProduction
The deployed API on 2.13.0 would fail every streamed Astra turn, because the NRE happens before the request is sent and the caller sees
AiUnavailable. Confirm live chat after this deploys. Flagged on the ticket as the urgent half.Nobody merged past a red gate
Worth stating, because it is the natural suspicion.
#479merged at 23:24:26Z, and the onlyUnit Testsrun on that head started at 23:27:56Z, three and a half minutes later, failing at 23:30:03Z. The failure did not exist at merge time.The missing push gate is the whole explanation: once the package was on
main, nothing re-ran the suite there, and the only signal was a job calledSonarCloud Analysiswhose name gives no hint that it is what runs the tests.