Fix review blockers in record chat

- Add Task.Supervisor to app supervision tree
- Use tagged tuples instead of rescue for error handling
- Pass embedding_text from parent LiveView as prop
- Remove dead Completion struct fields
- Extract socket assigns before Task closure
- Log errors server-side, show generic user message
- Make build_system_prompt private, remove thin wrapper
- Extract do_send_message for retry reuse

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Claudio Ortolina
2026-02-22 10:20:58 +00:00
parent 6e6e419801
commit 8d38512712
10 changed files with 99 additions and 91 deletions
+1
View File
@@ -23,6 +23,7 @@ defmodule MusicLibrary.Application do
{Oban, Application.fetch_env!(:music_library, Oban)}, {Oban, Application.fetch_env!(:music_library, Oban)},
{Ecto.Migrator, {Ecto.Migrator,
repos: Application.fetch_env!(:music_library, :ecto_repos), skip: skip_migrations?()}, repos: Application.fetch_env!(:music_library, :ecto_repos), skip: skip_migrations?()},
{Task.Supervisor, name: MusicLibrary.TaskSupervisor},
{Phoenix.PubSub, name: MusicLibrary.PubSub}, {Phoenix.PubSub, name: MusicLibrary.PubSub},
{LastFm.Supervisor, LastFm.Config.resolve(:music_library)}, {LastFm.Supervisor, LastFm.Config.resolve(:music_library)},
# Start a worker by calling: MusicLibrary.Worker.start_link(arg) # Start a worker by calling: MusicLibrary.Worker.start_link(arg)
+2 -6
View File
@@ -1,5 +1,5 @@
defmodule MusicLibrary.RecordChat do defmodule MusicLibrary.RecordChat do
alias MusicLibrary.Records.{Record, Similarity} alias MusicLibrary.Records.Record
def stream_response(messages, record, embedding_text, callback) do def stream_response(messages, record, embedding_text, callback) do
system_prompt = build_system_prompt(record, embedding_text) system_prompt = build_system_prompt(record, embedding_text)
@@ -11,11 +11,7 @@ defmodule MusicLibrary.RecordChat do
OpenAI.chat_stream(full_messages, on_chunk: callback) OpenAI.chat_stream(full_messages, on_chunk: callback)
end end
def get_embedding_text(record_id) do defp build_system_prompt(%Record{} = record, embedding_text) do
Similarity.get_embedding_text(record_id)
end
def build_system_prompt(%Record{} = record, embedding_text) do
context = context =
case embedding_text do case embedding_text do
nil -> basic_context(record) nil -> basic_context(record)
+55 -66
View File
@@ -1,6 +1,8 @@
defmodule MusicLibraryWeb.Components.RecordChat do defmodule MusicLibraryWeb.Components.RecordChat do
use MusicLibraryWeb, :live_component use MusicLibraryWeb, :live_component
require Logger
alias MusicLibrary.RecordChat alias MusicLibrary.RecordChat
def open(id), do: Fluxon.open_dialog(id) def open(id), do: Fluxon.open_dialog(id)
@@ -12,7 +14,6 @@ defmodule MusicLibraryWeb.Components.RecordChat do
|> assign(:messages, []) |> assign(:messages, [])
|> assign(:current_response, "") |> assign(:current_response, "")
|> assign(:loading, false) |> assign(:loading, false)
|> assign(:embedding_text, nil)
|> assign(:error, nil)} |> assign(:error, nil)}
end end
@@ -39,21 +40,7 @@ defmodule MusicLibraryWeb.Components.RecordChat do
end end
def update(assigns, socket) do def update(assigns, socket) do
{:ok, {:ok, assign(socket, assigns)}
socket
|> assign(assigns)
|> maybe_load_embedding_text()}
end
defp maybe_load_embedding_text(socket) do
if socket.assigns.embedding_text == nil and socket.assigns[:record] do
case RecordChat.get_embedding_text(socket.assigns.record.id) do
{:ok, text} -> assign(socket, :embedding_text, text)
{:error, :not_found} -> assign(socket, :embedding_text, nil)
end
else
socket
end
end end
@impl true @impl true
@@ -144,7 +131,12 @@ defmodule MusicLibraryWeb.Components.RecordChat do
</div> </div>
<div class="pt-4 border-t border-zinc-200 dark:border-zinc-700"> <div class="pt-4 border-t border-zinc-200 dark:border-zinc-700">
<form id={"#{@id}-form"} phx-submit="send_message" phx-target={@myself} class="flex gap-2"> <form
id={"#{@id}-form"}
phx-submit="send_message"
phx-target={@myself}
class="flex gap-2"
>
<.input <.input
name="message" name="message"
value="" value=""
@@ -188,51 +180,10 @@ defmodule MusicLibraryWeb.Components.RecordChat do
end end
@impl true @impl true
def handle_event("send_message", %{"message" => ""}, socket) do def handle_event("send_message", %{"message" => ""}, socket), do: {:noreply, socket}
{:noreply, socket}
end
def handle_event("send_message", %{"message" => text}, socket) do def handle_event("send_message", %{"message" => text}, socket),
parent_pid = self() do: do_send_message(socket, text)
component_id = socket.assigns.id
user_message = %{role: "user", content: String.trim(text)}
messages = socket.assigns.messages ++ [user_message]
Task.start(fn ->
try do
RecordChat.stream_response(
messages,
socket.assigns.record,
socket.assigns.embedding_text,
fn chunk ->
Phoenix.LiveView.send_update(parent_pid, __MODULE__,
id: component_id,
chunk: chunk
)
end
)
Phoenix.LiveView.send_update(parent_pid, __MODULE__,
id: component_id,
done: true
)
rescue
e ->
Phoenix.LiveView.send_update(parent_pid, __MODULE__,
id: component_id,
error: Exception.message(e)
)
end
end)
{:noreply,
socket
|> assign(:messages, messages)
|> assign(:loading, true)
|> assign(:current_response, "")
|> assign(:error, nil)}
end
def handle_event("clear_chat", _params, socket) do def handle_event("clear_chat", _params, socket) do
{:noreply, {:noreply,
@@ -245,17 +196,55 @@ defmodule MusicLibraryWeb.Components.RecordChat do
def handle_event("retry", _params, socket) do def handle_event("retry", _params, socket) do
case List.last(socket.assigns.messages) do case List.last(socket.assigns.messages) do
%{role: "user"} = last_message -> %{role: "user"} = last_message ->
# Remove the last user message and re-send socket
messages = Enum.drop(socket.assigns.messages, -1) |> assign(:messages, Enum.drop(socket.assigns.messages, -1))
|> do_send_message(last_message.content)
socket = assign(socket, :messages, messages)
handle_event("send_message", %{"message" => last_message.content}, socket)
_ -> _ ->
{:noreply, assign(socket, :error, nil)} {:noreply, assign(socket, :error, nil)}
end end
end end
defp do_send_message(socket, text) do
parent_pid = self()
component_id = socket.assigns.id
user_message = %{role: "user", content: String.trim(text)}
messages = socket.assigns.messages ++ [user_message]
record = socket.assigns.record
embedding_text = socket.assigns.embedding_text
Task.Supervisor.start_child(MusicLibrary.TaskSupervisor, fn ->
case RecordChat.stream_response(messages, record, embedding_text, fn chunk ->
Phoenix.LiveView.send_update(parent_pid, __MODULE__,
id: component_id,
chunk: chunk
)
end) do
:ok ->
Phoenix.LiveView.send_update(parent_pid, __MODULE__,
id: component_id,
done: true
)
{:error, reason} ->
Logger.error("RecordChat streaming error: #{reason}")
Phoenix.LiveView.send_update(parent_pid, __MODULE__,
id: component_id,
error: gettext("Something went wrong. Please try again.")
)
end
end)
{:noreply,
socket
|> assign(:messages, messages)
|> assign(:loading, true)
|> assign(:current_response, "")
|> assign(:error, nil)}
end
defp message_classes("user") do defp message_classes("user") do
"ml-auto bg-blue-500 dark:bg-blue-600 text-white" "ml-auto bg-blue-500 dark:bg-blue-600 text-white"
end end
@@ -344,6 +344,7 @@
sheet_id="record-chat-sheet" sheet_id="record-chat-sheet"
module={MusicLibraryWeb.Components.RecordChat} module={MusicLibraryWeb.Components.RecordChat}
record={@record} record={@record}
embedding_text={@embedding_text}
/> />
<.structured_modal <.structured_modal
@@ -308,6 +308,7 @@
sheet_id="record-chat-sheet" sheet_id="record-chat-sheet"
module={MusicLibraryWeb.Components.RecordChat} module={MusicLibraryWeb.Components.RecordChat}
record={@record} record={@record}
embedding_text={nil}
/> />
<.structured_modal <.structured_modal
+5 -2
View File
@@ -16,12 +16,15 @@ defmodule OpenAI do
{:ok, result} {:ok, result}
end end
def chat_stream(messages, opts \\ []) do def chat_stream(messages, opts \\ []) when is_list(messages) do
model = Keyword.get(opts, :model, "gpt-4o-mini") model = Keyword.get(opts, :model, "gpt-4o-mini")
temperature = Keyword.get(opts, :temperature, 0.7) temperature = Keyword.get(opts, :temperature, 0.7)
on_chunk = Keyword.fetch!(opts, :on_chunk) on_chunk = Keyword.fetch!(opts, :on_chunk)
API.chat_stream(messages, model, temperature, api_key(), on_chunk) case API.chat_stream(messages, model, temperature, api_key(), on_chunk) do
:ok -> :ok
{:error, _reason} = error -> error
end
end end
def embeddings(text) do def embeddings(text) do
+23 -14
View File
@@ -75,20 +75,29 @@ defmodule OpenAI.API do
end end
end end
Req.post!("https://api.openai.com/v1/chat/completions", case Req.post("https://api.openai.com/v1/chat/completions",
receive_timeout: 30_000, receive_timeout: 30_000,
connect_options: [ connect_options: [
timeout: 5_000 timeout: 5_000
], ],
json: %{ json: %{
model: model, model: model,
messages: messages, messages: messages,
stream: true, stream: true,
temperature: temperature temperature: temperature
}, },
auth: {:bearer, api_key}, auth: {:bearer, api_key},
finch_request: fun finch_request: fun
) ) do
{:ok, %{status: status}} when status in 200..299 ->
:ok
{:ok, %{body: body}} ->
{:error, "OpenAI API error: #{inspect(body)}"}
{:error, exception} ->
{:error, "Connection error: #{Exception.message(exception)}"}
end
end end
def get_embeddings(text, api_key) do def get_embeddings(text, api_key) do
+1 -3
View File
@@ -3,7 +3,5 @@ defmodule OpenAI.Completion do
defstruct content: "", defstruct content: "",
temperature: 0.2, temperature: 0.2,
role: "user", role: "user",
model: "gpt-4o-mini", model: "gpt-4o-mini"
messages: nil,
response_format: nil
end end
+5
View File
@@ -2000,3 +2000,8 @@ msgstr ""
#, elixir-autogen, elixir-format #, elixir-autogen, elixir-format
msgid "e.g. \"What genre is this?\", \"Tell me about the artists\"" msgid "e.g. \"What genre is this?\", \"Tell me about the artists\""
msgstr "" msgstr ""
#: lib/music_library_web/components/record_chat.ex
#, elixir-autogen, elixir-format
msgid "Something went wrong. Please try again."
msgstr ""
+5
View File
@@ -2000,3 +2000,8 @@ msgstr ""
#, elixir-autogen, elixir-format #, elixir-autogen, elixir-format
msgid "e.g. \"What genre is this?\", \"Tell me about the artists\"" msgid "e.g. \"What genre is this?\", \"Tell me about the artists\""
msgstr "" msgstr ""
#: lib/music_library_web/components/record_chat.ex
#, elixir-autogen, elixir-format
msgid "Something went wrong. Please try again."
msgstr ""