Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Built-in providers never populate reasoning, and the modified public initializer selectors break source compatibility.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 2
Open (3)
What changed in this PR
Adds display-only cumulative reasoning to session responses and streaming snapshots.
Changes:
- Propagates reasoning through response collection and structured wrappers.
- Prevents canceled partial streams from committing empty responses.
- Adds reasoning and cancellation regression tests.
| File | Description |
|---|---|
LanguageModelSession.swift |
Adds reasoning APIs, propagation, and cancellation handling. |
LocalGenerationUsage.swift |
Preserves reasoning in structured snapshots. |
ReasoningTests.swift |
Tests reasoning behavior and cancellation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Hi @qoli. Thanks for this, and for the tests, especially the one that caught a cancelled stream committing a partial response. Before we go further, can you tell us how you're using We'd also like whatever we add here to line up with Foundation Models 27, which represents reasoning as a transcript entry, Could you also split the cancellation change into its own PR? It's a good catch, but it changes behavior for every cancelled stream, not only ones with reasoning: skipping the commit also drops |
| The Anthropic adapter can replay its own reasoning entries after Codable restoration. | ||
| Other adapters currently reject reasoning replay explicitly rather than flattening | ||
| it into answer text or silently dropping it. For structured scalar outputs that |
|
Hi @qoli. Thank you for turning this around so quickly, and for being so flexible about the shape. Moving reasoning into the transcript, matching Foundation Models' Two things before we merge:
|
|
Thanks @mattt — addressed both points in 748539e.
Added deterministic streaming/nonstreaming request fixtures for Anthropic, OpenAI Chat Completions/Responses, OpenResponses, Gemini, and Ollama. They verify that unsupported reasoning/signatures are omitted without changing each adapter's existing request projection, and that Codable history remains intact. Validation: 595 default offline tests passed; 750 tests passed with the MLX, Llama, and CoreML traits. iOS Simulator build and strict formatting checks also passed. No live model calls were used for this verification. This change and the separate #266 refactor are now adopted together in my maintained fork ( |
| func appendAssistant(_ content: [AnthropicContent]) { | ||
| if let last = messages.last, last.role == .assistant { | ||
| messages[messages.count - 1] = .init(role: .assistant, content: last.content + content) | ||
| } else { | ||
| messages.append(.init(role: .assistant, content: content)) | ||
| } | ||
| } |
|
Hi @qoli. Sorry, I stepped on this with #271. It fixed the empty text block that Copilot found here, in the same case .response(let response):
// Anthropic rejects text blocks without non-whitespace text,
// such as the empty response of a turn that only called tools.
let content = convertSegmentsToAnthropicContent(response.segments).filter { block in
guard case .text(let text) = block else { return true }
return !text.text.allSatisfy(\.isWhitespace)
}
guard !content.isEmpty else { continue }
appendAssistant(content)With that, the full test suite passes for me locally. Copilot's other open note, about Core ML dropping reasoning, is the behavior I asked for, so you can ignore it. Once you've merged |


Reasoning belongs in the transcript, separate from person-facing answer content. This revision replaces the original
Response.reasoning/Snapshot.reasoningproposal with the Foundation Models 27 shape:Transcript.Entry.reasoning(Transcript.Reasoning), containing a stable ID, segments, opaqueData?signature, and metadata. Existing cumulativetranscriptEntriescarry it; response and snapshot initializer selectors remain unchanged.Use case
I maintain AIReasoningCore and SwiftChat. AIReasoningCore implements a custom
PiAILanguageModel: it maps pi-ai-swift's normalized reasoning events and terminal reasoning blocks into AnyLanguageModel sessions. SwiftChat displays provider-supplied reasoning in a separate collapsible area while answer text is still empty, accumulates it across tool rounds, and persists/restores it with the conversation. Reasoning transcript entries meet this use case and also retain the opaque state needed for provider replay. No separate response property or alternate session abstraction is needed.The initial PR only exposed a field for custom models. This version also implements the built-in Anthropic path: thinking and redacted-thinking entries in streaming and nonstreaming responses, stable streamed IDs, opaque signature preservation, and replay after Codable transcript restoration. Redacted payloads have no display segments. A README example shows configuring Anthropic thinking and reading reasoning and answer separately.
Scope
main. The session diff here is documentation only.main(0626c0f) without rewriting the existing PR branch history.Review follow-up and validation
Addresses the review in #264 (comment): unsupported replay no longer blocks model switching, and the initializer-compatibility test uses a watchOS 27 availability guard. The new provider-request fixtures compare against each adapter's existing no-reasoning request projection; original Codable history remains intact.
MLX,Llama,CoreMLtraits: 750 tests in 69 suites passed.Live-provider credentials were removed and
CI=1was used. No live model requests were made. The watchOS result is compilation of the actual tests, not just the library.