Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/agents/_data_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def __init__(

self._message_queue = self._conversation.frontend
self._thread = None
self._is_canceled = False # TODO modified by frontend
self._is_canceled = False

def get_initial_message(self):
if self._widget_config.initial_message:
Expand All @@ -116,6 +116,7 @@ def get_initial_message(self):
}

def post_user_message(self, user_message: str):
self._is_canceled = False
if not self._thread or not self._thread.is_alive():
while not self._message_queue.empty():
try:
Expand Down Expand Up @@ -156,6 +157,9 @@ def get_configuration(self):
def get_combined_tools_workflow_info(self):
return self._get_combined_tools_workflow_info

def cancel_agent(self):
self._is_canceled = True

# called by java, not the frontend
def get_view_data(self):

Expand Down Expand Up @@ -192,6 +196,8 @@ def _post_user_message(self, user_message: str):
self._handle_iteration_limit_error()
except Exception as e:
self._conversation.append_error(e)
finally:
self._is_canceled = False

def _handle_iteration_limit_error(self):
from langchain_core.messages import AIMessage
Expand Down
47 changes: 0 additions & 47 deletions src/agents/chat_app/dist/assets/index-BgQ0MdB3.js

This file was deleted.

Large diffs are not rendered by default.

47 changes: 47 additions & 0 deletions src/agents/chat_app/dist/assets/index-LVlEMt5c.js

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/agents/chat_app/dist/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
<link rel="icon" href="./favicon.png" type="image/png" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>AI Chat Assistant</title>
<script type="module" crossorigin src="./assets/index-BgQ0MdB3.js"></script>
<link rel="stylesheet" crossorigin href="./assets/index-CixTWtOl.css">
<script type="module" crossorigin src="./assets/index-LVlEMt5c.js"></script>
<link rel="stylesheet" crossorigin href="./assets/index-DLP8jyRf.css">
</head>
<body>
<div id="app"></div>
Expand Down
32 changes: 25 additions & 7 deletions src/agents/chat_app/src/components/chat/MessageInput.vue
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
<script setup lang="ts">
import { computed } from "vue";
import { computed, ref } from "vue";
import { useTextareaAutosize } from "@vueuse/core";

import { FunctionButton } from "@knime/components";
import SendIcon from "@knime/styles/img/icons/paper-flier.svg";
import AbortIcon from "@knime/styles/img/icons/close.svg";

import { useChatStore } from "@/stores/chat";

Expand All @@ -13,7 +14,8 @@ const { textarea, input } = useTextareaAutosize();
const characterLimit = 5000;

const isInputValid = computed(() => input.value?.trim().length > 0);
const isDisabled = computed(() => !isInputValid.value || chatStore.isLoading);
const isDisabled = computed(() => chatStore.isInterrupted || (!isInputValid.value && !chatStore.isLoading));


const handleClick = (event: MouseEvent) => {
if (event.target === event.currentTarget) {
Expand All @@ -22,13 +24,17 @@ const handleClick = (event: MouseEvent) => {
};

const handleSubmit = () => {
chatStore.sendUserMessage(input.value);
input.value = "";
if (chatStore.isLoading) {
chatStore.cancelAgent();
} else {
chatStore.sendUserMessage(input.value);
input.value = "";
}
};

const handleKeyDown = (event: KeyboardEvent) => {
// enter: send message
if (event.key === "Enter" && !event.shiftKey && !isDisabled.value) {
if (event.key === "Enter" && !chatStore.isLoading && !event.shiftKey && !isDisabled.value) {
event.preventDefault();
handleSubmit();
}
Expand Down Expand Up @@ -60,7 +66,18 @@ const handleKeyDown = (event: KeyboardEvent) => {
:disabled="isDisabled"
@click="handleSubmit"
>
<SendIcon class="send-icon" aria-hidden="true" focusable="false" />
<AbortIcon
v-if="chatStore.isLoading"
class="abort-icon"
aria-hidden="true"
focusable="false"
/>
<SendIcon
v-else
class="send-icon"
aria-hidden="true"
focusable="false"
/>
</FunctionButton>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also separate this into two buttons.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, it's ok

</div>
</template>
Expand Down Expand Up @@ -102,7 +119,8 @@ const handleKeyDown = (event: KeyboardEvent) => {
& svg {
stroke: var(--knime-dove-gray);

&.send-icon {
& .send-icon,
& .abort-icon {
margin-left: -1px;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,32 @@ describe("MessageInput", () => {
expect(textarea.element).toBe(document.activeElement);
});

it("disables send button when empty or loading", async () => {
it("manages send button disabled state correctly", async () => {
const wrapper = createWrapper();
const chatStore = useChatStore();
const sendButton = wrapper.find(".send-button");

// Initially empty -> disabled
// Initially empty and not loading -> disabled
expect(sendButton.attributes("disabled")).toBeDefined();

// Set input value to non-empty
// Set store loading to true -> should be enabled even if empty (to allow aborting)
chatStore.isLoading = true;
await wrapper.vm.$nextTick();
expect(sendButton.attributes("disabled")).toBeUndefined();

// Set store loading to false -> disabled again
chatStore.isLoading = false;
await wrapper.vm.$nextTick();
expect(sendButton.attributes("disabled")).toBeDefined();

// Set input value to non-empty -> enabled
await wrapper.find("textarea").setValue("Hello");
await wrapper.vm.$nextTick();
expect(sendButton.attributes("disabled")).toBeUndefined();

// Set store loading to true should disable button
// Set isLoading to true and isInterrupted to true -> disabled (abort in progress)
chatStore.isLoading = true;
chatStore.isInterrupted = true;
await wrapper.vm.$nextTick();
expect(sendButton.attributes("disabled")).toBeDefined();
});
Expand All @@ -70,20 +81,35 @@ describe("MessageInput", () => {
expect(textarea.element.value).toBe("");
});

it("does not call sendUserMessage when button is disabled due to loading", async () => {
it("calls store cancelAgent when send button is clicked while loading", async () => {
const wrapper = createWrapper();
const chatStore = useChatStore();
const cancelAgentSpy = vi.spyOn(chatStore, "cancelAgent").mockResolvedValue();

chatStore.isLoading = true;
await wrapper.vm.$nextTick();

await wrapper.find(".send-button").trigger("click");

expect(cancelAgentSpy).toHaveBeenCalled();
});

it("does not call handleSubmit on Enter while loading", async () => {
const wrapper = createWrapper();
const chatStore = useChatStore();
const sendUserMessageSpy = vi.spyOn(chatStore, "sendUserMessage").mockResolvedValue();

const cancelAgentSpy = vi.spyOn(chatStore, "cancelAgent").mockResolvedValue();

chatStore.isLoading = true;
await wrapper.vm.$nextTick();

const textarea = wrapper.find("textarea");
await textarea.setValue("Message");
await textarea.trigger("keydown", { key: "Enter", shiftKey: false });

expect(sendUserMessageSpy).not.toHaveBeenCalled();
expect(textarea.element.value).toContain("Message");
expect(cancelAgentSpy).not.toHaveBeenCalled();
expect(textarea.element.value).toBe("Message");
});

it("calls store sendUserMessage and clears input on Enter key press without Shift", async () => {
Expand Down Expand Up @@ -125,10 +151,10 @@ describe("MessageInput", () => {
const textarea = wrapper.find("textarea");

chatStore.lastUserMessage = "Previous message";

// Ensure input is empty first
await textarea.setValue("");

await textarea.trigger("keydown", { key: "ArrowUp" });

expect(textarea.element.value).toBe("Previous message");
Expand All @@ -140,7 +166,7 @@ describe("MessageInput", () => {
const textarea = wrapper.find("textarea");

chatStore.lastUserMessage = "Previous message";

await textarea.setValue("Current message");
await textarea.trigger("keydown", { key: "ArrowUp" });

Expand Down
42 changes: 42 additions & 0 deletions src/agents/chat_app/src/stores/__tests__/chat.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ describe("chat store", () => {
config: null,
chatItems: [],
isLoading: false,
isInterrupted: false,
lastUserMessage: "",
jsonDataService: null,
initState: "idle",
Expand Down Expand Up @@ -334,6 +335,31 @@ describe("chat store", () => {
error,
);
});

it("cancelAgent sets isInterrupted and calls backend", async () => {
const { store } = setupStore();
store.jsonDataService = mockJsonDataService;
mockJsonDataService.data.mockResolvedValue({});

expect(store.isInterrupted).toBe(false);

await store.cancelAgent();

expect(store.isInterrupted).toBe(true);
expect(mockJsonDataService.data).toHaveBeenCalledWith({
method: "cancel_agent",
});
});

it("cancelAgent resets isInterrupted on failure", async () => {
const { store } = setupStore();
store.jsonDataService = mockJsonDataService;
const error = new Error("Network error");
mockJsonDataService.data.mockRejectedValue(error);

await expect(store.cancelAgent()).rejects.toThrow(error);
expect(store.isInterrupted).toBe(false);
});
});

describe("sendUserMessage", () => {
Expand Down Expand Up @@ -465,6 +491,20 @@ describe("chat store", () => {
expect(timeline.label).toBe("Completed 1 tool call");
});

it("resets isInterrupted when loading completes with AI message", () => {
const { store } = setupStore();
store.isLoading = true;
store.isInterrupted = true;
store.initState = "ready";
store.config = createConfig(true);

// Receiving a final AI message should trigger finishLoading
store.addMessages([createAiMessage("Final response")], true);

expect(store.isLoading).toBe(false);
expect(store.isInterrupted).toBe(false);
});

it("handles multiple tool calls in timeline label", () => {
const { store } = setupStore();
const activeTimeline = createTimeline("Using tools", "active", [
Expand Down Expand Up @@ -650,6 +690,7 @@ describe("chat store", () => {
store.config = config;
store.chatItems.push(userMessage);
store.isLoading = true;
store.isInterrupted = true;
store.lastUserMessage = "test";
store.jsonDataService = mockJsonDataService;

Expand All @@ -660,6 +701,7 @@ describe("chat store", () => {
config: null,
chatItems: [],
isLoading: false,
isInterrupted: false,
isUsingTools: false,
lastUserMessage: "",
jsonDataService: null,
Expand Down
22 changes: 20 additions & 2 deletions src/agents/chat_app/src/stores/chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ export const useChatStore = defineStore("chat", () => {
const lastMessage = shallowRef<Message | undefined>();
const config = ref<Config | null>(null); // node settings that affect rendering
const isLoading = ref(false); // true if agent is currently responding to user message
const isInterrupted = ref(false); // true if agent is currently responding to cancellation
const chatItems = ref<ChatItem[]>([]);
const lastUserMessage = ref("");
const jsonDataService = ref<JsonDataService | null>(null);
Expand Down Expand Up @@ -357,6 +358,19 @@ export const useChatStore = defineStore("chat", () => {
}
}

async function cancelAgent() {
try {
isInterrupted.value = true;
await jsonDataService.value?.data({
method: "cancel_agent",
});
} catch (error) {
isInterrupted.value = false;
consola.error("Chat Store: Failed to cancel agent:", error);
throw error;
}
}

async function postUserMessage(msg: string) {
try {
await jsonDataService.value?.data({
Expand Down Expand Up @@ -458,8 +472,8 @@ export const useChatStore = defineStore("chat", () => {
const lastMessagesToDisplay = showToolCallsResults
? msgs
: msgs.filter(
(msg) => !isToolMessage(msg) && !isAiMessageWithToolCalls(msg),
);
(msg) => !isToolMessage(msg) && !isAiMessageWithToolCalls(msg),
);
const activeTimeline = chatItems.value.findLast(
(item) => item.type === "timeline" && item.status === "active",
) as Timeline | undefined;
Expand All @@ -472,6 +486,7 @@ export const useChatStore = defineStore("chat", () => {

function finishLoading(shallApplyViewData: boolean) {
isLoading.value = false;
isInterrupted.value = false;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My strategy was to check where we reset isLoading and to also reset isInterrupted there. But we should double-check if this covers all cases.

const viewData: ViewData = {
conversation: messagesToPersist,
config: toRaw(config.value!),
Expand All @@ -495,6 +510,7 @@ export const useChatStore = defineStore("chat", () => {
config.value = null;
chatItems.value = [];
isLoading.value = false;
isInterrupted.value = false;
lastUserMessage.value = "";
jsonDataService.value = null;
initState.value = "idle";
Expand All @@ -507,6 +523,7 @@ export const useChatStore = defineStore("chat", () => {
chatItems,
lastMessage,
isLoading,
isInterrupted,
lastUserMessage,
jsonDataService,
initState,
Expand All @@ -532,5 +549,6 @@ export const useChatStore = defineStore("chat", () => {
flushRequestQueue,
pollForNewMessages,
resetChat,
cancelAgent,
};
});