Refactor localized month labels
This commit is contained in:
137
AUDIT.md
Normal file
137
AUDIT.md
Normal file
@@ -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.
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user