From e1abe457e8cabef9c84c6a5caa90fecce10ab28a Mon Sep 17 00:00:00 2001 From: Chili Palmer Date: Mon, 22 Jun 2026 11:16:47 +0200 Subject: [PATCH] feat: close out of AUDIT implementation --- AUDIT.md | 179 ------------------------------------------------------- 1 file changed, 179 deletions(-) delete mode 100644 AUDIT.md diff --git a/AUDIT.md b/AUDIT.md deleted file mode 100644 index dda406a..0000000 --- a/AUDIT.md +++ /dev/null @@ -1,179 +0,0 @@ -# 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` -- [x] `rendering/liquid_parser.ex` (verified Liquex does not expose a built-in filter/operator subset validator; keeping current implementation) -- [x] `rendering/filters.ex` -- [x] `rendering/post_rendering.ex` -- [x] `rendering/list_archive.ex` -- [x] `rendering/template_selection.ex` -- [x] `rendering/metadata.ex` -- [x] `rendering/links_and_languages.ex` -- [x] `generation/validation.ex` -- [x] `generation/outputs.ex` -- [x] `generation/sitemap.ex` -- [x] `generation/data.ex` -- [x] `generation/paths.ex` -- [x] `generation/renderers.ex` -- [x] `mcp/tools.ex` -- [x] `mcp/server.ex` -- [x] `ai/chat_tools.ex` -- [x] `ai/chat.ex` -- [x] `ai/openai_compatible_runtime.ex` -- [x] `ai/one_shot.ex` -- [x] `ai/secret_key.ex` -- [x] `ai/secret_backend.ex` -- [x] `scripting/capabilities/util.ex` -- [x] `desktop/automation.ex` (verified keep: current module is an external-process automation harness; replacing it with `Desktop.Window` primitives would not be a local simplification) -- [x] `desktop/shell_live/sidebar_events.ex` -- [x] `MapUtils.attr/2` replacements -- [x] `ui/registry.ex` O(1) lookups (verified keep: compile-time caching would freeze localized labels) -- [x] `bounded_atoms.ex` O(1) allow-lists -- [x] `scripting/job_store.ex` -- [x] `desktop/server.ex` -- [x] `desktop/shell_live/code_entity_editor.ex` -- [x] `scripting/runtime.ex` -- [x] `desktop/external_links.ex` -- [x] `MapUtils.maybe_put/3` replacements -- [x] `MapUtils.blank_to_nil/1` replacements (exact duplicate sites replaced; trim-sensitive local variants verified keep) -- [x] `desktop/error_html.ex` (verified keep: still required by Phoenix `render_errors` wiring) -- [x] `desktop/health_controller.ex` (verified keep: current Phoenix route wiring still expects a controller module) -- [x] `desktop/folder_picker.ex` (verified keep: no equivalent `Desktop.OS` picker primitive exists to inline onto) -- [x] `desktop/file_picker.ex` (verified keep: no equivalent `Desktop.OS` picker primitive exists to inline onto) -- [x] `desktop/deep_link.ex` (verified keep: this is an active GenServer bridge for OS URL events, not a trivial wrapper) -- [x] `desktop/overlay.ex` (verified keep: this is a real overlay state/transform module, not a one-function shim) -- [x] `rebuild.ex` (verified keep: shared `Task.async_stream` helper remains clearer than duplicating the concurrency options at call sites) -- [x] `starter_templates.ex` (verified keep: shared starter-template path/default helpers remain clearer than inlining across rendering/template code) - ---- - -## 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.