fix: fixed duplicate elements in ai chat
This commit is contained in:
@@ -6,6 +6,7 @@ defmodule BDS.Desktop.ShellLive.ChatEditor do
|
|||||||
|
|
||||||
alias BDS.AI
|
alias BDS.AI
|
||||||
alias BDS.MapUtils
|
alias BDS.MapUtils
|
||||||
|
alias BDS.Persistence
|
||||||
alias BDS.Desktop.ShellData
|
alias BDS.Desktop.ShellData
|
||||||
alias BDS.Desktop.ShellLive.ChatEditor.{MessageBuild, ModelSelection, ToolTracking}
|
alias BDS.Desktop.ShellLive.ChatEditor.{MessageBuild, ModelSelection, ToolTracking}
|
||||||
|
|
||||||
@@ -125,6 +126,7 @@ defmodule BDS.Desktop.ShellLive.ChatEditor do
|
|||||||
|
|
||||||
true ->
|
true ->
|
||||||
live_view_pid = self()
|
live_view_pid = self()
|
||||||
|
started_at = Persistence.now_ms()
|
||||||
|
|
||||||
task =
|
task =
|
||||||
Task.Supervisor.async_nolink(BDS.Tasks.TaskSupervisor, fn ->
|
Task.Supervisor.async_nolink(BDS.Tasks.TaskSupervisor, fn ->
|
||||||
@@ -146,6 +148,7 @@ defmodule BDS.Desktop.ShellLive.ChatEditor do
|
|||||||
Map.put(socket.assigns.chat_editor_requests, conversation_id, %{
|
Map.put(socket.assigns.chat_editor_requests, conversation_id, %{
|
||||||
ref: task.ref,
|
ref: task.ref,
|
||||||
pid: task.pid,
|
pid: task.pid,
|
||||||
|
started_at: started_at,
|
||||||
message: message,
|
message: message,
|
||||||
content: "",
|
content: "",
|
||||||
tool_events: []
|
tool_events: []
|
||||||
@@ -200,6 +203,7 @@ defmodule BDS.Desktop.ShellLive.ChatEditor do
|
|||||||
[
|
[
|
||||||
%{
|
%{
|
||||||
type: :call,
|
type: :call,
|
||||||
|
id: ToolTracking.tool_call_id(tool_call),
|
||||||
name: ToolTracking.tool_call_name(tool_call),
|
name: ToolTracking.tool_call_name(tool_call),
|
||||||
arguments: ToolTracking.tool_call_arguments(tool_call)
|
arguments: ToolTracking.tool_call_arguments(tool_call)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -17,6 +17,8 @@ defmodule BDS.Desktop.ShellLive.ChatEditor.MessageBuild do
|
|||||||
request = Map.get(assigns.chat_editor_requests, conversation.id)
|
request = Map.get(assigns.chat_editor_requests, conversation.id)
|
||||||
effective_model = AI.effective_chat_model(conversation)
|
effective_model = AI.effective_chat_model(conversation)
|
||||||
available_models = AI.available_chat_models(effective_model)
|
available_models = AI.available_chat_models(effective_model)
|
||||||
|
streaming_tool_markers = streaming_tool_markers(messages, request)
|
||||||
|
streaming_content = streaming_content(messages, request)
|
||||||
|
|
||||||
%{
|
%{
|
||||||
id: conversation.id,
|
id: conversation.id,
|
||||||
@@ -31,9 +33,10 @@ defmodule BDS.Desktop.ShellLive.ChatEditor.MessageBuild do
|
|||||||
messages: build_entries(messages, assigns),
|
messages: build_entries(messages, assigns),
|
||||||
pending_user_message: pending_user_message(messages, request),
|
pending_user_message: pending_user_message(messages, request),
|
||||||
is_streaming: not is_nil(request),
|
is_streaming: not is_nil(request),
|
||||||
streaming_content: streaming_content(request),
|
streaming_content: streaming_content,
|
||||||
streaming_tool_markers: ToolTracking.tool_markers_from_events(request),
|
streaming_tool_markers: streaming_tool_markers,
|
||||||
streaming_inline_surfaces: streaming_inline_surfaces(conversation.id, request, assigns),
|
streaming_inline_surfaces:
|
||||||
|
streaming_inline_surfaces(conversation.id, streaming_tool_markers, assigns),
|
||||||
offline?: Map.get(assigns, :offline_mode, true),
|
offline?: Map.get(assigns, :offline_mode, true),
|
||||||
needs_api_key?: ModelSelection.needs_api_key?(Map.get(assigns, :offline_mode, true)),
|
needs_api_key?: ModelSelection.needs_api_key?(Map.get(assigns, :offline_mode, true)),
|
||||||
action_error: Map.get(assigns.chat_editor_action_errors, conversation.id),
|
action_error: Map.get(assigns.chat_editor_action_errors, conversation.id),
|
||||||
@@ -137,28 +140,135 @@ defmodule BDS.Desktop.ShellLive.ChatEditor.MessageBuild do
|
|||||||
|
|
||||||
defp pending_user_message(_messages, nil), do: nil
|
defp pending_user_message(_messages, nil), do: nil
|
||||||
|
|
||||||
defp pending_user_message(messages, %{message: message}) when is_binary(message) do
|
defp pending_user_message(messages, %{message: message} = request) when is_binary(message) do
|
||||||
|
cond do
|
||||||
|
persisted_user_message_for_request?(messages, request) ->
|
||||||
|
nil
|
||||||
|
|
||||||
|
true ->
|
||||||
|
legacy_pending_user_message(messages, message)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
defp pending_user_message(_messages, _request), do: nil
|
||||||
|
|
||||||
|
defp legacy_pending_user_message(messages, message) do
|
||||||
case messages |> Enum.reverse() |> Enum.find(&(&1.role not in [:system, :tool])) do
|
case messages |> Enum.reverse() |> Enum.find(&(&1.role not in [:system, :tool])) do
|
||||||
%{role: :user, content: ^message} -> nil
|
%{role: :user, content: ^message} -> nil
|
||||||
_other -> message
|
_other -> message
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
defp pending_user_message(_messages, _request), do: nil
|
|
||||||
|
|
||||||
defp streaming_content(nil), do: ""
|
defp streaming_content(nil), do: ""
|
||||||
defp streaming_content(%{content: content}) when is_binary(content), do: content
|
defp streaming_content(%{content: content}) when is_binary(content), do: content
|
||||||
defp streaming_content(_request), do: ""
|
defp streaming_content(_request), do: ""
|
||||||
|
|
||||||
defp streaming_inline_surfaces(_conversation_id, nil, _assigns), do: []
|
defp streaming_content(messages, request) do
|
||||||
|
content = streaming_content(request)
|
||||||
|
|
||||||
defp streaming_inline_surfaces(conversation_id, request, assigns) do
|
if content != "" and persisted_assistant_content_for_request?(messages, request, content) do
|
||||||
|
""
|
||||||
|
else
|
||||||
|
content
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
defp streaming_tool_markers(_messages, nil), do: []
|
||||||
|
|
||||||
|
defp streaming_tool_markers(messages, request) do
|
||||||
request
|
request
|
||||||
|> ToolTracking.tool_markers_from_events()
|
|> ToolTracking.tool_markers_from_events()
|
||||||
|
|> drop_persisted_tool_markers(messages, request)
|
||||||
|
end
|
||||||
|
|
||||||
|
defp streaming_inline_surfaces(_conversation_id, [], _assigns), do: []
|
||||||
|
|
||||||
|
defp streaming_inline_surfaces(conversation_id, tool_markers, assigns) do
|
||||||
|
tool_markers
|
||||||
|> ToolSurfaces.build_render_surfaces("streaming-#{conversation_id}", assigns)
|
|> ToolSurfaces.build_render_surfaces("streaming-#{conversation_id}", assigns)
|
||||||
|> mark_surfaces_expanded(assigns)
|
|> mark_surfaces_expanded(assigns)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
defp persisted_user_message_for_request?(messages, %{message: message} = request)
|
||||||
|
when is_binary(message) do
|
||||||
|
messages
|
||||||
|
|> persisted_messages_for_request(request)
|
||||||
|
|> Enum.any?(fn persisted_message ->
|
||||||
|
persisted_message.role == :user and persisted_message.content == message
|
||||||
|
end)
|
||||||
|
end
|
||||||
|
|
||||||
|
defp persisted_user_message_for_request?(_messages, _request), do: false
|
||||||
|
|
||||||
|
defp persisted_assistant_content_for_request?(messages, request, content)
|
||||||
|
when is_binary(content) and content != "" do
|
||||||
|
messages
|
||||||
|
|> persisted_messages_for_request(request)
|
||||||
|
|> Enum.any?(fn persisted_message ->
|
||||||
|
persisted_message.role == :assistant and (persisted_message.content || "") == content
|
||||||
|
end)
|
||||||
|
end
|
||||||
|
|
||||||
|
defp persisted_assistant_content_for_request?(_messages, _request, _content), do: false
|
||||||
|
|
||||||
|
defp drop_persisted_tool_markers(tool_markers, messages, request) do
|
||||||
|
persisted_markers = persisted_tool_markers_for_request(messages, request)
|
||||||
|
|
||||||
|
{remaining, _persisted_markers} =
|
||||||
|
Enum.reduce(tool_markers, {[], persisted_markers}, fn marker, {remaining, persisted_markers} ->
|
||||||
|
case pop_matching_tool_marker(persisted_markers, marker) do
|
||||||
|
{nil, persisted_markers} -> {remaining ++ [marker], persisted_markers}
|
||||||
|
{_matched, persisted_markers} -> {remaining, persisted_markers}
|
||||||
|
end
|
||||||
|
end)
|
||||||
|
|
||||||
|
remaining
|
||||||
|
end
|
||||||
|
|
||||||
|
defp persisted_tool_markers_for_request(messages, request) do
|
||||||
|
messages
|
||||||
|
|> persisted_messages_for_request(request)
|
||||||
|
|> Enum.flat_map(fn message ->
|
||||||
|
if message.role == :assistant do
|
||||||
|
ToolTracking.normalize_tool_calls(message.tool_calls)
|
||||||
|
else
|
||||||
|
[]
|
||||||
|
end
|
||||||
|
end)
|
||||||
|
end
|
||||||
|
|
||||||
|
defp pop_matching_tool_marker(tool_markers, marker) do
|
||||||
|
case Enum.find_index(tool_markers, &same_tool_marker?(&1, marker)) do
|
||||||
|
nil -> {nil, tool_markers}
|
||||||
|
index -> {Enum.at(tool_markers, index), List.delete_at(tool_markers, index)}
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
defp same_tool_marker?(left, right) do
|
||||||
|
cond do
|
||||||
|
is_binary(left.id) and is_binary(right.id) ->
|
||||||
|
left.id == right.id
|
||||||
|
|
||||||
|
true ->
|
||||||
|
left.name == right.name and (left.arguments || %{}) == (right.arguments || %{})
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
defp persisted_messages_for_request(messages, request) do
|
||||||
|
case request_started_at(request) do
|
||||||
|
started_at when is_integer(started_at) ->
|
||||||
|
Enum.filter(messages, fn message ->
|
||||||
|
is_integer(message.created_at) and message.created_at >= started_at
|
||||||
|
end)
|
||||||
|
|
||||||
|
_other ->
|
||||||
|
[]
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
defp request_started_at(%{started_at: started_at}) when is_integer(started_at), do: started_at
|
||||||
|
defp request_started_at(_request), do: nil
|
||||||
|
|
||||||
defp translated(text, bindings \\ %{}),
|
defp translated(text, bindings \\ %{}),
|
||||||
do: ShellData.translate(text, bindings, BDS.Desktop.UILocale.current())
|
do: ShellData.translate(text, bindings, BDS.Desktop.UILocale.current())
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -8,6 +8,14 @@ defmodule BDS.Desktop.ShellLive.ChatEditor.ToolTracking do
|
|||||||
BDS.MapUtils.attr(tool_call, :name) || "tool"
|
BDS.MapUtils.attr(tool_call, :name) || "tool"
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@spec tool_call_id(term()) :: term()
|
||||||
|
def tool_call_id(tool_call) when is_map(tool_call) do
|
||||||
|
BDS.MapUtils.attr(tool_call, :id)
|
||||||
|
end
|
||||||
|
|
||||||
|
@spec tool_call_id(term()) :: term()
|
||||||
|
def tool_call_id(_tool_call), do: nil
|
||||||
|
|
||||||
@spec tool_call_arguments(term()) :: term()
|
@spec tool_call_arguments(term()) :: term()
|
||||||
def tool_call_arguments(tool_call) when is_map(tool_call) do
|
def tool_call_arguments(tool_call) when is_map(tool_call) do
|
||||||
BDS.MapUtils.attr(tool_call, :arguments) || BDS.MapUtils.attr(tool_call, :args) || %{}
|
BDS.MapUtils.attr(tool_call, :arguments) || BDS.MapUtils.attr(tool_call, :args) || %{}
|
||||||
@@ -72,7 +80,7 @@ defmodule BDS.Desktop.ShellLive.ChatEditor.ToolTracking do
|
|||||||
markers ++
|
markers ++
|
||||||
[
|
[
|
||||||
%{
|
%{
|
||||||
id: nil,
|
id: Map.get(event, :id),
|
||||||
name: event.name,
|
name: event.name,
|
||||||
arguments: event.arguments,
|
arguments: event.arguments,
|
||||||
args_preview: tool_arguments_preview(event.arguments || %{}),
|
args_preview: tool_arguments_preview(event.arguments || %{}),
|
||||||
|
|||||||
@@ -3331,6 +3331,95 @@ defmodule BDS.Desktop.ShellLiveTest do
|
|||||||
refute render(view) =~ "Delayed response"
|
refute render(view) =~ "Delayed response"
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "chat editor does not duplicate persisted turn artifacts while the request is still active" do
|
||||||
|
assert :ok = AI.set_airplane_mode(false)
|
||||||
|
|
||||||
|
server =
|
||||||
|
start_supervised!({Bandit, plug: DelayedChatServer, port: 0, startup_log: false})
|
||||||
|
|
||||||
|
{:ok, {_address, port}} = ThousandIsland.listener_info(server)
|
||||||
|
|
||||||
|
assert {:ok, _endpoint} =
|
||||||
|
AI.put_endpoint(:online, %{
|
||||||
|
url: "http://127.0.0.1:#{port}/v1",
|
||||||
|
api_key: "online-secret",
|
||||||
|
model: "gpt-4.1"
|
||||||
|
})
|
||||||
|
|
||||||
|
assert {:ok, conversation} = AI.start_chat(%{title: "Streaming Dedupe", model: "gpt-4.1"})
|
||||||
|
|
||||||
|
{:ok, view, _html} = live_isolated(build_conn(), BDS.Desktop.ShellLive)
|
||||||
|
|
||||||
|
_html =
|
||||||
|
render_click(view, "pin_sidebar_item", %{
|
||||||
|
"route" => "chat",
|
||||||
|
"id" => conversation.id,
|
||||||
|
"title" => conversation.title,
|
||||||
|
"subtitle" => conversation.model || "chat"
|
||||||
|
})
|
||||||
|
|
||||||
|
_html = render_change(view, "change_chat_editor_input", %{"message" => "Newest question"})
|
||||||
|
|
||||||
|
_html =
|
||||||
|
view
|
||||||
|
|> element("[data-testid='chat-send-button']")
|
||||||
|
|> render_click()
|
||||||
|
|
||||||
|
assert Enum.count(AI.list_chat_messages(conversation.id), fn message ->
|
||||||
|
message.role == :user and message.content == "Newest question"
|
||||||
|
end) == 1
|
||||||
|
|
||||||
|
now = Persistence.now_ms()
|
||||||
|
|
||||||
|
Repo.insert!(
|
||||||
|
BDS.AI.ChatMessage.changeset(%BDS.AI.ChatMessage{}, %{
|
||||||
|
conversation_id: conversation.id,
|
||||||
|
role: :assistant,
|
||||||
|
content: "",
|
||||||
|
tool_calls:
|
||||||
|
Jason.encode!([
|
||||||
|
%{
|
||||||
|
"id" => "call-card-new",
|
||||||
|
"name" => "render_card",
|
||||||
|
"arguments" => %{
|
||||||
|
"title" => "Latest Missing Data",
|
||||||
|
"body" => "The second data request needs review."
|
||||||
|
}
|
||||||
|
}
|
||||||
|
]),
|
||||||
|
created_at: now + 1
|
||||||
|
})
|
||||||
|
)
|
||||||
|
|
||||||
|
send(view.pid, {
|
||||||
|
:chat_tool_call,
|
||||||
|
conversation.id,
|
||||||
|
%{
|
||||||
|
id: "call-card-new",
|
||||||
|
name: "render_card",
|
||||||
|
arguments: %{
|
||||||
|
"title" => "Latest Missing Data",
|
||||||
|
"body" => "The second data request needs review."
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
html = render(view)
|
||||||
|
|
||||||
|
refute html =~ ~s(data-testid="chat-pending-user-message")
|
||||||
|
assert length(:binary.matches(html, ~s(data-testid="chat-user-message-text"))) == 1
|
||||||
|
assert length(:binary.matches(html, ~s(data-testid="chat-inline-surface"))) == 1
|
||||||
|
refute html =~ ~s(data-testid="chat-streaming-message")
|
||||||
|
assert html =~ ~s(data-testid="chat-streaming-thinking")
|
||||||
|
|
||||||
|
_html =
|
||||||
|
view
|
||||||
|
|> element("[data-testid='chat-abort-button']")
|
||||||
|
|> render_click()
|
||||||
|
|
||||||
|
Process.sleep(350)
|
||||||
|
end
|
||||||
|
|
||||||
test "translation validation route renders dedicated cards and fix controls", %{
|
test "translation validation route renders dedicated cards and fix controls", %{
|
||||||
project: project,
|
project: project,
|
||||||
temp_dir: temp_dir
|
temp_dir: temp_dir
|
||||||
|
|||||||
Reference in New Issue
Block a user