Skip to content

Commit de6d616

Browse files
chat: fix Responses API tool-chaining, context limits, and streaming correctness (#1080)
## Summary Three rounds of review against the [Azure OpenAI Responses API docs](https://learn.microsoft.com/en-us/azure/foundry/openai/how-to/responses) (using Claude Opus 4.6 and GPT-5.5 as parallel review subagents) identified and fixed several correctness bugs and robustness gaps in the AI chat implementation. --- ## Critical fixes ### C1 — Parallel streaming tool calls created forked conversation branches Previously, `ProcessStreamingUpdatesAsync` fired a separate API continuation per tool call inline during the stream. With N parallel tool calls this created N independent conversation branches — each continuation only saw its own tool result, not the others. **Fix:** Two-phase pattern — buffer all `FunctionCallResponseItem`s during the stream, execute them sequentially after the stream completes, then make **one** continuation with all outputs. Matches the non-streaming behavior. ### C2 — Context length errors not caught in streaming path `ClientResultException` from a context-window overflow propagates through `await foreach` but C# prohibits `yield return` inside a `try/catch` block (CS1626). **Fix:** Added `RethrowContextLengthErrors` helper that puts `try/catch` only around `MoveNextAsync`, with `yield return` outside — the only CS1626-safe pattern for exception remapping in async iterators. --- ## High fixes ### H1 — SSE error event lost on mid-stream exception The second `catch (Exception ex)` block in `StreamMessage` was missing its `try { await Response.WriteAsync(...) } catch {}` body, so clients received no signal when the stream was interrupted. ### H2 — `CloneOptionsWithPreviousResponseId` copied only 3 of 14 fields Tool-call continuation legs inherited `Instructions`, `PreviousResponseId`, and `Tools` — but not `EndUserId`, `Temperature`, `TopP`, `TruncationMode`, `MaxOutputTokenCount`, `TextOptions`, `ParallelToolCallsEnabled`, `StoredOutputEnabled`, `ToolChoice`, `ServiceTier`, or `Metadata`. Continuations were silently using defaults for all of those. ### H3 — Context length detection relied on substring matching only **Fix:** `IsContextLengthError` now parses the structured JSON error body via `TryExtractErrorCode` and checks the `error.code` field first (`context_length_exceeded`, `token_limit_exceeded`). Also handles HTTP 413. Falls back to message substring matching. --- ## Medium fixes (from second review pass) - **`ParseToolArguments` unguarded in non-streaming path** — a `JsonException` on malformed model-generated arguments would bubble up as a 500. Now caught identically to the streaming path, returning an error `FunctionCallOutputResponseItem` so the model can recover. - **`strictModeEnabled: false` for MCP tool registration** — the MCP .NET SDK generates schemas that don't satisfy OpenAI strict-mode constraints (optional params not in `required`, no `additionalProperties: false`, nullable union types). A single non-conforming schema with `strictModeEnabled: true` causes a 400 that blocks all tools. - **`currentLegResponseId` null guard** — if the API never emits `StreamingResponseCreatedUpdate`, the continuation would fire with `PreviousResponseId = null`, causing a confusing 400. Now throws `InvalidOperationException` with a diagnostic message. - **Log original exception in mid-stream context-limit handler** — was logging a freshly constructed `InvalidOperationException`, discarding the original stack trace and `PreviousResponseId`. --- ## Bonus - **`EndUserId` wired up** — `CreateResponseOptionsAsync` now sets `options.EndUserId` (the SDK exposes it; the previous code discarded it). Enables Azure OpenAI abuse monitoring and Defender prompt-shield correlation. - **`LogMcpToolArgumentParseError`** logger message added. - **`ConversationContextLimitExceededException`** domain exception with `PreviousResponseId` property. --- ## Files changed | File | Changes | |------|---------| | `EssentialCSharp.Chat.Shared/Services/AIChatService.cs` | All service-level fixes | | `EssentialCSharp.Chat.Shared/Services/ConversationContextLimitExceededException.cs` | New domain exception | | `EssentialCSharp.Web/Controllers/ChatController.cs` | Context-limit handling, SSE error write, logging |
1 parent 9a3a1d5 commit de6d616

3 files changed

Lines changed: 335 additions & 129 deletions

File tree

0 commit comments

Comments
 (0)