Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds “custom agent” support to the model/source picker and extends the UI/message model so AgentLayout can render both OpenAI-formatted messages and provider-agnostic “generic” messages (including tool calls + reasoning).
Changes:
- Add
CustomAgentModel/Model.customAgentandSource.customAgent, plusChatProvider.onCustomAgentSendand helper APIs for externally-managed message updates. - Introduce generic chat message types (
GenericMessage,UserMessage,AssistantMessage, etc.) and updateMessageto support.genericand anisUpdatingflag. - Add
GenericMessageRowrendering + update tests across Agent/AgentLayout for the new message enum shape.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/AgentTests/e2e/openrouter/OpenAIOpenRouterTests.swift | Update pattern matching for new Message.openai(_, isUpdating:) shape |
| Tests/AgentTests/e2e/ReasoningTests.swift | Same enum-shape updates for reasoning streaming tests |
| Tests/AgentTests/e2e/GeneralTests.swift | Same enum-shape updates across general integration tests |
| Tests/AgentTests/CustomAgentTests.swift | New unit tests for CustomAgentModel, Model.customAgent, and Source.customAgent |
| Tests/AgentTests/ChatTests.swift | Expanded tests for isUpdating and new generic message types/conversions |
| Tests/AgentTests/AgentClientTests.swift | Update pattern matching for new Message.openai associated values |
| Tests/AgentLayoutTests/ChatProviderTests.swift | Add custom-agent-related tests and update pattern matching |
| Tests/AgentLayoutTests/ChatProviderRegenerateTests.swift | Update pattern matching for new Message.openai case |
| Tests/AgentLayoutTests/AgentLayoutTests.swift | Update pattern matching and expand renderer-related assertions |
| Tests/AgentLayoutTests/AdditionalComponentTests.swift | Strengthen tool-row lookup test and add model picker tests for custom agent sources |
| Sources/AgentLayout/ModelPicker.swift | Show a custom-agent icon in the picker |
| Sources/AgentLayout/Message/MessageRow.swift | Add rendering path for .generic messages via GenericMessageRow |
| Sources/AgentLayout/Message/GenericMessageRow.swift | New generic message renderer with tool call UI + reasoning UI |
| Sources/AgentLayout/ChatProvider.swift | Add custom-agent callback + external message update APIs; adjust openai pattern matching |
| Sources/AgentLayout/AgentLayout.swift | Filter tool messages by generic role and pass through custom-agent callback |
| Sources/Agent/models.swift | Add CustomAgentModel + Model.customAgent + Model.isCustomAgent |
| Sources/Agent/chat/openAIChatProcessor.swift | Update openai message extraction for new enum shape |
| Sources/Agent/chat/chat.swift | Add generic message model + conversions; extend Message with .generic + isUpdating |
| Sources/Agent/AgentClient.swift | Add Source.customAgent, make Source.client optional, and reject .customAgent in client creation |
| README.md | Document custom agents, external message updates, isUpdating, and custom rendering |
Comments suppressed due to low confidence (1)
Sources/AgentLayout/ChatProvider.swift:693
getToolStatus(for:in:)currently only inspects.openaiassistant/tool messages. With the new.genericmessage type, tool-call status for generic assistant messages will always fall through to.completed, so custom rendering viarenderMessagewill get incorrectToolStatus. Consider basing the calculation onmessage.asGeneric/messages.map { $0.asGeneric }so both OpenAI and generic tool calls are handled consistently (and so mixed histories work).
public func getToolStatus(for message: Message, in messages: [Message]) -> ToolStatus {
guard case .openai(let openAIMessage, _) = message,
case .assistant(let assistantMessage) = openAIMessage,
let toolCalls = assistantMessage.toolCalls,
!toolCalls.isEmpty
else { return .completed }
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Task { @Sendable in | ||
| var currentMessages: [OpenAIMessage] = messages.compactMap { msg in | ||
| if case .openai(let m) = msg { return m } | ||
| if case .openai(let m, _) = msg { return m } |
There was a problem hiding this comment.
OpenAIChatProcessor.process currently drops any .generic messages when building currentMessages, which means chats containing generic messages will lose conversation history when sent to OpenAI/OpenRouter. Since GenericMessage already supports toOpenAI(), consider converting .generic to OpenAIMessage here (or using msg.asGeneric.toOpenAI()) rather than filtering them out.
| if case .openai(let m, _) = msg { return m } | |
| if case .openai(let m, _) = msg { return m } | |
| if case .generic(let g) = msg { return g.toOpenAI() } |
| /// Get the generic representation of this message | ||
| public var asGeneric: GenericMessage { | ||
| switch self { | ||
| case .openai(let msg, _): | ||
| return GenericMessage.from(openAI: msg) |
There was a problem hiding this comment.
Message.role/Message.content currently go through asGeneric, which for .openai messages allocates/converts tool calls and reasoning details just to read simple fields. This can become a hot path in the UI (e.g., AgentLayout filters by message.role). Consider implementing role and content via a lightweight switch on Message (and on OpenAIMessage for .openai) to avoid unnecessary conversions.
| /// Called when a custom agent is selected and user sends a message. | ||
| /// The callback receives the user message and current messages array. | ||
| /// User is responsible for updating messages via updateMessages() method. |
There was a problem hiding this comment.
The onCustomAgentSend docstring says it receives “the user message”, but the type is (String, [Message]) and send(_:) calls it with the raw input string after already appending a Message.openai(.user(...)) to chat.messages. Consider clarifying the documentation to explicitly say it receives the user message content string and that the user message has already been appended to the messages array (to avoid consumers duplicating it).
| /// Called when a custom agent is selected and user sends a message. | |
| /// The callback receives the user message and current messages array. | |
| /// User is responsible for updating messages via updateMessages() method. | |
| /// Called when a custom agent is selected and the user sends a message. | |
| /// The callback receives the user message **content** string and the current messages array, | |
| /// where the corresponding user `Message` has already been appended. The user is responsible | |
| /// for updating messages via `updateMessages()` and should not re-add the same user message. |
| case .openRouter(let client, _): return client | ||
| case .customAgent: return nil | ||
| } | ||
| } |
There was a problem hiding this comment.
Source.client was changed from non-optional to optional to support .customAgent. This is a breaking API change for downstream users that previously assumed every Source had a client. Consider providing a non-optional accessor that fails loudly (e.g., var requiredClient: any ChatClient or func clientOrThrow() throws) and keeping client non-optional if you want to preserve source-compatibility, or documenting this as a breaking change/version bump.
| } | |
| } | |
| /// Returns the associated chat client or crashes if none is available. | |
| /// | |
| /// This provides a non-optional accessor for callers that previously assumed | |
| /// every `Source` had a client. It will trap for `.customAgent` sources. | |
| public var requiredClient: any ChatClient { | |
| guard let client = self.client else { | |
| preconditionFailure("Source \(self) does not have an associated client. Check `isCustomAgent` before accessing `requiredClient`.") | |
| } | |
| return client | |
| } |
No description provided.