diff --git a/AUDIT.md b/AUDIT.md new file mode 100644 index 0000000..6a7917e --- /dev/null +++ b/AUDIT.md @@ -0,0 +1,137 @@ +# Ponytail Audit: bDS2 Codebase + +Scope: over-engineering, dead code, hand-rolled stdlib, yagni. Out of scope: correctness, +security, performance. + +Estimated reduction: ~2,100 lines deleted, 0 deps removed. + +**Rules:** One public module per file is idiomatic Elixir — never merge schemas or contexts +to lower file count (saves zero lines, fights the language). "Savings" = lines deleted, not +relocated. Work top-down; items are ordered highest-gain/lowest-risk first. Run `mix test` +after each item. + +## Progress + +- [x] `rendering/labels.ex` + +--- + +## 1. Shrink verbose modules (~2,000 lines) + +Behaviour-preserving refactors that delete duplicated render/build patterns and hand-rolled +lookups. This is the bulk of real savings. + +| Module | Lines | Target | What to cut | +|--------|-------|--------|-------------| +| `rendering/labels.ex` | 91 | ~30 | 28 month-name clauses → list + `Enum.at` | +| `rendering/liquid_parser.ex` | 150 | ~80 | Hand-rolled AST walk for filter/operator validation — only if Liquex exposes a built-in (verify first; if not, keep) | +| `rendering/filters.ex` | 627 | ~450 | Deduplicate macro rendering pattern | +| `rendering/post_rendering.ex` | 290 | ~200 | Pipeline inline variable assignments | +| `rendering/list_archive.ex` | 352 | ~280 | Deduplicate post rendering pattern | +| `rendering/template_selection.ex` | 197 | ~150 | Simplify query logic | +| `rendering/metadata.ex` | 138 | ~100 | Merge menu item builder | +| `rendering/links_and_languages.ex` | 139 | ~100 | Simplify path builders | +| `generation/validation.ex` | 509 | ~350 | Deduplicate timestamp check builders | +| `generation/outputs.ex` | 598 | ~450 | Simplify route path builders | +| `generation/sitemap.ex` | 287 | ~200 | Simplify URL entry builders | +| `generation/data.ex` | 409 | ~320 | Simplify snapshot accumulator | +| `generation/paths.ex` | 238 | ~180 | Deduplicate path builders | +| `generation/renderers.ex` | 174 | ~120 | Simplify fallback rendering | +| `mcp/tools.ex` | 718 | ~500 | Deduplicate tool call patterns | +| `mcp/server.ex` | 346 | ~250 | Simplify TCP server loop | +| `ai/chat_tools.ex` | 1105 | ~800 | Deduplicate tool query patterns | +| `ai/chat.ex` | 992 | ~750 | Simplify streaming state machine | +| `ai/openai_compatible_runtime.ex` | 301 | ~200 | Merge streaming + non-streaming paths | +| `ai/one_shot.ex` | 563 | ~400 | Deduplicate prompt builder pattern | +| `ai/secret_key.ex` | 233 | ~150 | Simplify key resolution chain | +| `ai/secret_backend.ex` | 111 | ~80 | Simplify legacy key fallback | +| `scripting/capabilities/util.ex` | 200 | ~120 | Merge one_arg/two_arg/three_arg into a higher-order fn | +| `desktop/automation.ex` | 333 | ~200 | Use `Desktop.Window` primitives directly | +| `desktop/shell_live/sidebar_events.ex` | 191 | ~50 | Replace 13 handle clauses with a dispatch map | + +## 2. Deduplicate private utilities into `BDS.MapUtils` (~50 lines) + +`MapUtils.attr/2` already has 124 callers — it is canonical. Remove the scattered private copies: + +1. `defp attr/2` → `BDS.MapUtils.attr(attrs, key)`. Files: `tags.ex`, `metadata.ex`, `tasks.ex`, `projects.ex`, `menu.ex`, `import_definitions.ex` +2. `defp maybe_put/3` → `BDS.MapUtils.maybe_put/3`. Files: `tags.ex`, `import_analysis.ex`, `chat_tools.ex`, `import_editor.ex`, `import_definitions.ex`, `analysis_state.ex`, `taxonomy_editing.ex`. For `""`-guard variants (`tags.ex`, `chat_tools.ex`): `BDS.MapUtils.maybe_put(map, key, if(value == "", do: nil, else: value))` +3. `defp blank_to_nil/1` → `BDS.MapUtils.blank_to_nil/1`. ~15 locations across shell_live editors, settings editors, maintenance, import_analysis. For the `"\n"`-guard variant in `git.ex`, inline that guard at the call site. + +Keep `map_utils.ex` — it is the canonical home. + +## 3. O(1) lookups for static data + +### 3a. `lib/bds/ui/registry.ex` +`sidebar_view/1` and `editor_route/1` do `Enum.find` over per-call-rebuilt lists. +```elixir +@sidebar_view_by_id Map.new(sidebar_views(), &{&1.id, &1}) +@editor_route_by_id Map.new(editor_routes(), &{&1.id, &1}) +def sidebar_view(id), do: @sidebar_view_by_id[id] +def editor_route(id), do: @editor_route_by_id[id] +``` + +### 3b. `lib/bds/bounded_atoms.ex` +`sidebar_views/0` / `editor_routes/0` recompute `Enum.map(Registry...)` per call. Make them attributes: +```elixir +@sidebar_view_ids Enum.map(Registry.sidebar_views(), & &1.id) +@editor_route_ids Enum.map(Registry.editor_routes(), & &1.id) +defp sidebar_views, do: @sidebar_view_ids +defp editor_routes, do: @editor_route_ids +``` + +## 4. GenServer → lighter primitive + +### 4a. `lib/bds/scripting/job_store.ex` → `Agent` +Pure state container: every `handle_call` is a Map read/write, no `handle_info`, no side effects. +```elixir +defmodule BDS.Scripting.JobStore do + use Agent + def start_link(_), do: Agent.start_link(fn -> %{jobs: %{}, runners: %{}} end, name: __MODULE__) + def get, do: Agent.get(__MODULE__, & &1) + def update(fun), do: Agent.update(__MODULE__, fun) +end +``` +Keep public function names; route them through `Agent.get`/`Agent.update`. + +### 4b. `lib/bds/desktop/server.ex` → Bandit child spec +Wraps `Bandit.start_link/1` in a GenServer and stores a PID it never reads. Supervise Bandit directly: +```elixir +def child_spec(opts) do + Supervisor.child_spec( + {Bandit, plug: BDS.Desktop.Endpoint, scheme: :http, ip: {127, 0, 0, 1}, port: port(), startup_log: false}, + id: __MODULE__ + ) +end +``` +Keep `port/0` and `url/0` as plain module functions. + +## 5. Delete dead / single-impl code + +- `lib/bds/desktop/shell_live/code_entity_editor.ex` — 0 lines, verified empty. +- `lib/bds/scripting/runtime.ex` — behaviour with one impl (`Lua`). Fold the contract into `lua.ex`, update the alias in `scripting.ex`. + +Verify each call site before deleting. + +## 6. Inline one-function wrapper files + +Inline at the call site, delete the file, `mix test`. One at a time — verify call sites. + +| File | Lines | Replacement | +|------|-------|-------------| +| `lib/bds/desktop/error_html.ex` | 9 | inline `"not found"` into `router.ex` | +| `lib/bds/desktop/health_controller.ex` | 9 | inline `"ok"` into `router.ex` | +| `lib/bds/desktop/external_links.ex` | 12 | `Desktop.OS.open_url` at call site | +| `lib/bds/desktop/folder_picker.ex` | 35 | `Desktop.OS` directly | +| `lib/bds/desktop/file_picker.ex` | 35 | `Desktop.OS` directly | +| `lib/bds/desktop/deep_link.ex` | 35 | inline at call site | +| `lib/bds/desktop/overlay.ex` | 35 | `Desktop.Window` directly | +| `lib/bds/rebuild.ex` | 23 | `Task.async_stream` at call site | +| `lib/bds/starter_templates.ex` | 22 | inline into `templates.ex` | + +Keep (idiomatic, not over-engineering): `lib/bds.ex` (app entry / namespace root), +`lib/bds/slug.ex` (inline only if a single caller of `BDS.Slug` — grep first), +`lib/bds/desktop/menu_compat.ex` (shared `__using__` macro for `Menu` + `MenuBar`). + +--- + +Final gate after all items: `mix test`, `mix credo --strict`, `mix deps.audit`, `mix dialyzer`. Warnings are errors. diff --git a/lib/bds/rendering/labels.ex b/lib/bds/rendering/labels.ex index 3e665e6..b711655 100644 --- a/lib/bds/rendering/labels.ex +++ b/lib/bds/rendering/labels.ex @@ -39,52 +39,24 @@ defmodule BDS.Rendering.Labels do @spec month_name(integer() | nil, String.t()) :: String.t() | nil def month_name(nil, _language), do: nil - def month_name(1, language) do - Gettext.with_locale(BDS.Gettext, language, fn -> dgettext("render", "January") end) - end - - def month_name(2, language) do - Gettext.with_locale(BDS.Gettext, language, fn -> dgettext("render", "February") end) - end - - def month_name(3, language) do - Gettext.with_locale(BDS.Gettext, language, fn -> dgettext("render", "March") end) - end - - def month_name(4, language) do - Gettext.with_locale(BDS.Gettext, language, fn -> dgettext("render", "April") end) - end - - def month_name(5, language) do - Gettext.with_locale(BDS.Gettext, language, fn -> dgettext("render", "May") end) - end - - def month_name(6, language) do - Gettext.with_locale(BDS.Gettext, language, fn -> dgettext("render", "June") end) - end - - def month_name(7, language) do - Gettext.with_locale(BDS.Gettext, language, fn -> dgettext("render", "July") end) - end - - def month_name(8, language) do - Gettext.with_locale(BDS.Gettext, language, fn -> dgettext("render", "August") end) - end - - def month_name(9, language) do - Gettext.with_locale(BDS.Gettext, language, fn -> dgettext("render", "September") end) - end - - def month_name(10, language) do - Gettext.with_locale(BDS.Gettext, language, fn -> dgettext("render", "October") end) - end - - def month_name(11, language) do - Gettext.with_locale(BDS.Gettext, language, fn -> dgettext("render", "November") end) - end - - def month_name(12, language) do - Gettext.with_locale(BDS.Gettext, language, fn -> dgettext("render", "December") end) + def month_name(month, language) when month in 1..12 do + Gettext.with_locale(BDS.Gettext, language, fn -> + [ + dgettext("render", "January"), + dgettext("render", "February"), + dgettext("render", "March"), + dgettext("render", "April"), + dgettext("render", "May"), + dgettext("render", "June"), + dgettext("render", "July"), + dgettext("render", "August"), + dgettext("render", "September"), + dgettext("render", "October"), + dgettext("render", "November"), + dgettext("render", "December") + ] + |> Enum.at(month - 1) + end) end def month_name(_month, _language), do: nil