Bug/ap 24779 agent chat view doesnt respond warning#57
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “warning” message type to support a frontend warning banner (e.g., when model credentials are unavailable) and updates the agent chat widget backend to avoid intermediate AI-message validation during message appends, validating only the final AI message instead.
Changes:
- Add
warningas a first-class message type across the chat app types, store, UI, and tests, and render it via a newWarningBannercomponent. - Fetch and surface startup warnings from the backend during chat initialization, and allow dismissing them without polluting the chat timeline.
- Backend: detect credential/model initialization issues early, send a startup warning to the frontend, and bypass intermediate AI-message validation during conversation appends (validating only the final AI message).
Reviewed changes
Copilot reviewed 10 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/agents/chat_app/src/types.ts | Adds warning message type and WarningMessage to the message union. |
| src/agents/chat_app/src/test/factories/messages.ts | Adds a warning-message test factory matcher. |
| src/agents/chat_app/src/stores/chat.ts | Stores latest warning separately, fetches startup warnings, and filters warnings out of timeline persistence/display. |
| src/agents/chat_app/src/stores/tests/chat.test.ts | Adds store tests for warning handling and adjusts init mocks for new startup fetch. |
| src/agents/chat_app/src/components/chat/WarningBanner.vue | New warning banner UI component with dismiss action. |
| src/agents/chat_app/src/components/chat/tests/WarningBanner.test.ts | Unit tests for warning banner rendering and dismiss emission. |
| src/agents/chat_app/src/components/chat/ChatInterface.vue | Renders WarningBanner when a warning is present in the store. |
| src/agents/chat_app/src/components/chat/tests/ChatInterface.test.ts | Tests banner visibility and dismiss wiring to the store action. |
| src/agents/chat_app/dist/index.html | Updates built asset references (hashed JS/CSS). |
| src/agents/chat_app/dist/assets/index-exEeaaxF.css | Updates built CSS (includes warning banner styles). |
| src/agents/base.py | Adds startup credential/model init error detection and passes startup_error into the data service. |
| src/agents/_data_service.py | Queues startup warnings, handles “no agent” case on message post, and bypasses intermediate AI-message validation during append. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def append_warning_to_frontend(self, warning: Union[Exception, str]): | ||
| """Appends a warning only to the frontend.""" | ||
| content = str(warning) | ||
|
|
||
| warning_message = {"type": "warning", "content": content} | ||
| self._frontend.put(warning_message) |
There was a problem hiding this comment.
append_warning_to_frontend enqueues a warning dict without an id. In the frontend, WarningMessage extends BaseMessage and requires id: string (and several tests/matchers assume an id is present). Consider generating a stable unique id for warnings (e.g., uuid/monotonic counter) and including it in the payload to keep the message shape consistent with other message types.
| for new_message in messages: | ||
| self._backend_messages._append(new_message) # _backend_messages.append_messages does validation | ||
|
|
There was a problem hiding this comment.
FrontendConversation._append_messages calls the private AgentPrompterConversation._append(...) to bypass validation. This works today, but it tightly couples the data service to a private method and makes future refactors of AgentPrompterConversation riskier. Consider adding an explicit public API on AgentPrompterConversation for appending without validation (or a flag on append_messages) and use that instead of reaching into _append.
| for msg in messages: | ||
| self._backend_messages._append(msg) # _backend_messages.append_messages does validation |
There was a problem hiding this comment.
Same coupling issue in append_messages_to_backend: it iterates messages and calls the private _backend_messages._append(...). Prefer using a dedicated public method on AgentPrompterConversation for unvalidated appends so this behavior is intentional and less brittle.
| for msg in messages: | |
| self._backend_messages._append(msg) # _backend_messages.append_messages does validation | |
| # Use the backend's public API for appending messages instead of its private `_append` method. | |
| self._backend_messages.append_messages(messages) |
Adds a new warning banner in the frontend, which is used to indicate if credentials are unavailable. Warnings are a new message type that can be "send" from the backend.
There is also one commit that fixes a bug in the agent chat widget conversation. Previously, when messages where appended, it relied on the Agent Prompter Conversation function which also validated all AI messages (as a result, validation errors were not handled properly). The new commit avoids this validation. We validate the final AI message after the agent loop anyways.