[T0ha]

Summary

• Enforce a single selected LLM config with a migration and partial unique index.
• Add transactional model selection and a clear-selection path that restores openrouter/free.
• Update the admin UI and OpenRouter requests to use one selected model.
• Document the model selection workflow and add coverage for selection, fallback, and UI state.

Validation

• mix compile: passed (existing Telegram webhook warning remains)
• mix format --check-formatted: passed
• iex -S mix: started and exited; database connection warnings because PostgreSQL is unavailable on localhost:5433
• mix test: blocked because PostgreSQL is unavailable on localhost:5433
• mix dialyzer: reports two unreachable error pattern warnings in LLM config creation flows
• mix precommit: stops at strict Credo due existing repository findings

[claude]

Code Review

Reviewed the diff for the single-selected-model feature (migration, context functions, OpenRouter request building, and admin LiveView). Overall the design is sound — transactional selection guarded by a partial unique index is a good approach — but I found one crash-level regression that should block merge, plus a few smaller items.

Critical: create_llm_config/1 crashes on any validation error

lib/bodhi/llm_configs.ex:306

defp maybe_invalidate_cache({:ok, _}, _attrs), do: :ok

The old code had a second, catch-all clause (defp maybe_invalidate_cache(_, _), do: :ok) that was dropped in this PR. Now create_llm_config/1 (lib/bodhi/llm_configs.ex:111-121) calls maybe_invalidate_cache(result, attrs) unconditionally, and when Repo.insert/1 returns {:error, changeset} (missing required fields, duplicate name/model, etc.) there is no matching function clause, so it raises FunctionClauseError instead of returning {:error, changeset}.

This breaks:

• The New LLM Config form's error path (BodhiWeb.LlmConfigLive.Form.save_config/3 expects {:error, %Ecto.Changeset{}} to re-render inline errors) — it would crash the LiveView instead.
• sync_models/1's create_new_models/2 (lib/bodhi/llm_configs.ex:238), which pattern-matches on {:error, _} to skip failed creations — a real DB constraint violation during sync would crash the whole sync instead of being skipped.
• The PR's own new/updated tests: "with invalid data returns error changeset" and "enforces unique name constraint" in test/bodhi/llm_configs_test.exs both call create_llm_config with attrs that produce {:error, changeset}, so they should fail with this crash rather than pass.

This is likely why it wasn't caught — the PR description notes mix test couldn't run locally because Postgres wasn't reachable. Worth restoring the catch-all clause (or just inlining the {:ok, _} check without a second private function) before merging.

Confusing error changeset for "model no longer exists"

lib/bodhi/llm_configs.ex:77-82

Repo.rollback(
  LlmConfig.changeset(%LlmConfig{}, %{})
  |> Ecto.Changeset.add_error(:id, "model does not exist")
)

LlmConfig.changeset(%LlmConfig{}, %{}) already fails validate_required for name/model/position, so the returned changeset carries three unrelated "can't be blank" errors alongside the real "model does not exist" error. The LiveView then renders inspect(changeset.errors) directly in the flash (lib/bodhi_web/live/llm_config_live/index.ex:362), which would show confusing, irrelevant validation noise to the admin if they select a model that was just deleted by someone else. Consider returning a plain error atom/tuple instead of a changeset here, or building an empty, unvalidated changeset just to carry the one error.

Minor: unguarded String.to_integer/1 on client-supplied id

lib/bodhi_web/live/llm_config_live/index.ex:352

def handle_event("select_model", %{"id" => id}, socket) do
  case LlmConfigs.select_llm_config(String.to_integer(id)) do

A tampered/non-numeric id in the event payload (e.g. via a manually crafted pushEvent in devtools) raises ArgumentError and crashes the LiveView process rather than failing gracefully. Not exploitable beyond a self-inflicted crash of the admin's own session, but Integer.parse/1 with a fallback error branch would be more robust, consistent with how delete handles the id via Repo.get!.

Nit: terminology inconsistency in the table

The status badge changed from "Active" to "Selected" (lib/bodhi_web/live/llm_config_live/index.ex:179), but the table column header and the filter dropdown (active/inactive in the select) still say "Active"/"Inactive". Not a bug, just a small polish opportunity now that "active" is user-facing as "selected".

Nice touches

• The partial unique index + transactional select_llm_config/1 correctly prevents two active rows even under concurrent selection.
• Test updates consistently adapt existing fixtures to the new single-active invariant.
• The README.md fix for :ai_client to :llm_provider corrects docs that were already stale relative to config/config.exs, good catch even though out of the PR's core scope.

🤖 Generated with Claude Code