diff --git a/CODESMELL.md b/CODESMELL.md index 4957ad3..72928ef 100644 --- a/CODESMELL.md +++ b/CODESMELL.md @@ -99,11 +99,9 @@ _None._ All modules previously on the queue have been split; refresh the queue i ## 9. `Jason.decode!/1` on External HTTP Responses -**Status:** open. +**Status:** ✅ done (2026-05-01). The 2 scoped HTTP response decodes in `BDS.AI.OpenAICompatibleRuntime` now use `Jason.decode/1` and return `{:error, %{kind: :invalid_json_response, reason: reason}}` for malformed model-list or chat-completion response bodies. The error shape propagates through endpoint model listing, chat, and one-shot orchestration via the existing `with` chains. Remaining `Jason.decode!/1` sites in `BDS.AI.Catalog` decode model-catalog HTTP/cache payloads or trusted on-disk JSON and are out of scope for this section. -**Scope:** only the 2 sites in `BDS.AI` / `BDS.AI.OpenAICompatibleRuntime` that decode HTTP response bodies. The remaining 12 `Jason.decode!/1` sites decode our own on-disk files (metadata, embeddings index, generation hashes) and are not in scope. - -**Plan:** switch to `Jason.decode/1` and propagate `{:error, reason}` from the runtime through to the chat / one-shot orchestration layer in `BDS.AI.Chat` and `BDS.AI.OneShot`. +**Rule:** OpenAI-compatible runtime HTTP responses must not be decoded with bang functions; malformed provider JSON is a recoverable runtime error. --- @@ -161,6 +159,10 @@ Most tests share the SQLite repo and named GenServers (`BDS.Tasks`, `BDS.Search` ## Changelog +### 2026-05-01 + +- **`Jason.decode!/1` on external HTTP responses**: replaced the 2 scoped OpenAI-compatible runtime response decodes with `Jason.decode/1` and tagged `{:error, %{kind: :invalid_json_response, reason: reason}}` propagation for malformed `/models` and `/chat/completions` bodies. Added regressions covering endpoint model listing through a fake HTTP client and generation through a local Bandit server. Section 9 is closed. + ### 2026-05-10 - **Duplicate helpers across contexts**: added `BDS.MapUtils` (`attr/2`, `maybe_put/3`, `blank_to_nil/1`) and expanded `BDS.ProgressReporter` (`callback/1`, `scaled/3`, `report_count_started/4`, `report_count_progress/5`, `report_rebuild_started/3`, `report_rebuild_progress/4`, `report_phase/3`). Replaced copy-pasted helpers in posts, post translations, post rebuild, media file ops, search, generation progress, maintenance progress, embeddings/index progress, publishing, MCP util, scripts, and templates. Domain-specific modules now express their wording/ranges as options to the shared reporter, preserving existing user-facing progress messages while sharing one implementation. Added focused tests for both shared modules. Section 6 is closed. diff --git a/lib/bds/ai/openai_compatible_runtime.ex b/lib/bds/ai/openai_compatible_runtime.ex index c70a01d..7014d3f 100644 --- a/lib/bds/ai/openai_compatible_runtime.ex +++ b/lib/bds/ai/openai_compatible_runtime.ex @@ -47,23 +47,24 @@ defmodule BDS.AI.OpenAICompatibleRuntime do end defp normalize_response(body) do - payload = Jason.decode!(body) - message = get_in(payload, ["choices", Access.at(0), "message"]) || %{} - content = normalize_content(message["content"]) - tool_calls = normalize_tool_calls(message["tool_calls"] || []) - usage = normalize_usage(payload["usage"] || %{}) + with {:ok, payload} <- decode_json_body(body) do + message = get_in(payload, ["choices", Access.at(0), "message"]) || %{} + content = normalize_content(message["content"]) + tool_calls = normalize_tool_calls(message["tool_calls"] || []) + usage = normalize_usage(payload["usage"] || %{}) - json = - case content do - nil -> nil - value when is_binary(value) -> - case Jason.decode(value) do - {:ok, decoded} when is_map(decoded) -> decoded - _other -> nil - end - end + json = + case content do + nil -> nil + value when is_binary(value) -> + case Jason.decode(value) do + {:ok, decoded} when is_map(decoded) -> decoded + _other -> nil + end + end - {:ok, %{content: content, json: json, tool_calls: tool_calls, usage: usage}} + {:ok, %{content: content, json: json, tool_calls: tool_calls, usage: usage}} + end end defp completions_url(url) do @@ -84,24 +85,31 @@ defmodule BDS.AI.OpenAICompatibleRuntime do end defp normalize_models_response(body) do - payload = Jason.decode!(body) + with {:ok, payload} <- decode_json_body(body) do + models = + payload + |> Map.get("data", []) + |> Enum.map(fn entry -> + id = entry["id"] || entry[:id] - models = - payload - |> Map.get("data", []) - |> Enum.map(fn entry -> - id = entry["id"] || entry[:id] + %{ + id: id, + label: id + } + end) + |> Enum.reject(&is_nil(&1.id)) + |> Enum.uniq_by(& &1.id) + |> Enum.sort_by(&String.downcase(&1.id)) - %{ - id: id, - label: id - } - end) - |> Enum.reject(&is_nil(&1.id)) - |> Enum.uniq_by(& &1.id) - |> Enum.sort_by(&String.downcase(&1.id)) + {:ok, models} + end + end - {:ok, models} + defp decode_json_body(body) do + case Jason.decode(body) do + {:ok, payload} -> {:ok, payload} + {:error, reason} -> {:error, %{kind: :invalid_json_response, reason: reason}} + end end defp maybe_put_auth(headers, nil), do: headers diff --git a/test/bds/ai_test.exs b/test/bds/ai_test.exs index 071dd18..edb49d6 100644 --- a/test/bds/ai_test.exs +++ b/test/bds/ai_test.exs @@ -88,6 +88,25 @@ defmodule BDS.AITest do def get(_url, _headers), do: {:error, :not_found} end + defmodule BadJsonEndpointHttpClient do + def get("https://api.example.test/v1/models", _headers) do + {:ok, %{status: 200, headers: %{}, body: "not json"}} + end + end + + defmodule BadJsonCompletionServer do + use Plug.Router + + plug(:match) + plug(:dispatch) + + post "/v1/chat/completions" do + conn + |> Plug.Conn.put_resp_content_type("application/json") + |> Plug.Conn.send_resp(200, "not json") + end + end + defmodule FakeRuntime do def generate(endpoint, request, opts) do test_pid = Keyword.fetch!(opts, :test_pid) @@ -256,6 +275,32 @@ defmodule BDS.AITest do assert [%{id: "gpt-4.1", label: "gpt-4.1"}, %{id: "gpt-4.1-mini", label: "gpt-4.1-mini"}] = models end + test "list_endpoint_models returns an error for malformed endpoint JSON" do + assert {:error, %{kind: :invalid_json_response, reason: %Jason.DecodeError{}}} = + BDS.AI.list_endpoint_models(%{url: "https://api.example.test/v1", api_key: "online-secret"}, + http_client: BadJsonEndpointHttpClient + ) + end + + test "openai-compatible generation returns an error for malformed completion JSON" do + server = + start_supervised!({Bandit, plug: BadJsonCompletionServer, port: 0, startup_log: false}) + + {:ok, {_address, port}} = ThousandIsland.listener_info(server) + + assert {:error, %{kind: :invalid_json_response, reason: %Jason.DecodeError{}}} = + BDS.AI.OpenAICompatibleRuntime.generate( + %{url: "http://127.0.0.1:#{port}/v1", api_key: nil}, + %{ + model: "gpt-test", + messages: [%{"role" => "user", "content" => "Hello"}], + max_output_tokens: 128, + tools: [] + }, + [] + ) + end + test "airplane mode routes title tasks to airplane endpoint and offline title model" do assert {:ok, _endpoint} = BDS.AI.put_endpoint(:online, %{