Compare commits
4 Commits
f2b340ba86
...
a5ac74db91
| Author | SHA1 | Date | |
|---|---|---|---|
| a5ac74db91 | |||
| beca4d992f | |||
| 9e6d93a4b3 | |||
| e29dfb490a |
49
CODESMELL.md
49
CODESMELL.md
@@ -500,30 +500,49 @@
|
||||
|
||||
---
|
||||
|
||||
### CSM-033 — `Enum.each` with Side Effects That Should Be Batch Inserts
|
||||
- **Files:** `lib/bds/search.ex:174-177`, `lib/bds/embeddings.ex`
|
||||
- **What:** `Enum.each` used for inserting records. The side-effect pattern is fine, but `Enum.map` + `Repo.insert_all` would be much faster for bulk inserts.
|
||||
- **Fix:** Use `Repo.insert_all` for batch inserts instead of `Enum.each` + `Repo.insert`.
|
||||
### ~~CSM-033 — `Enum.each` with Side Effects That Should Be Batch Inserts~~ ✅ FIXED
|
||||
- **Fixed:** 2026-05-27
|
||||
- **What was done:**
|
||||
- **`lib/bds/search.ex`** — Already addressed by CSM-006; `batch_insert_post_index` and `batch_insert_media_index` use multi-row SQL INSERT with chunking.
|
||||
- **`lib/bds/embeddings.ex`** — Replaced `Enum.each` + per-post `sync_post_if_enabled` (which did N individual `Repo.get_by` reads + N individual `Repo.insert_or_update` writes) in three bulk functions:
|
||||
- `rebuild_project/2` — preloads all keys via `preload_keys_by_post_id/1`, computes rows with `compute_key_data/3`, batch-upserts with `batch_upsert_keys/1`.
|
||||
- `repair_posts/2` — same pattern with `preload_keys_by_post_id/2` scoped to target post IDs.
|
||||
- `index_unindexed/1` — same pattern, eliminating per-post `Repo.get_by` lookups.
|
||||
- Added `preload_keys_by_post_id/1` and `/2` — single-query key preload into a map by post_id.
|
||||
- Added `max_label_value/0` — reads max label once instead of per-post `next_label()` queries.
|
||||
- Added `compute_key_data/3` — resolves body, hashes, embeds (if needed), returns `:skip` or `{:upsert, row}`.
|
||||
- Added `batch_upsert_keys/1` — multi-row `INSERT INTO embedding_keys ... ON CONFLICT(label) DO UPDATE` with 199-row chunking (SQLite 999-param limit ÷ 5 columns).
|
||||
- `sync_post_if_enabled/2` retained for single-post `sync_post/1` path (CRUD operations).
|
||||
- Added 11 tests in `test/bds/csm033_batch_inserts_test.exs`: source-level assertions (no Enum.each+sync_post_if_enabled, batch_upsert_keys present, preload present, ON CONFLICT upsert, compute_key_data used), search.ex batch verification, and functional tests (index 5 posts, rebuild updates stale keys, repair targets subset, skip on matching hash).
|
||||
|
||||
---
|
||||
|
||||
### CSM-034 — `File.read!` / `File.write!` Without Error Handling
|
||||
- **Files:** `lib/bds/preview_assets.ex:32`, `lib/bds/release_packaging.ex:105`, `lib/bds/templates.ex:488-489`
|
||||
- **Fix:** Use `File.read/1`, `File.write/2`, and handle `{:error, reason}`.
|
||||
### ~~CSM-034 — `File.read!` / `File.write!` Without Error Handling~~ ✅ FIXED
|
||||
- **Fixed:** 2026-05-27
|
||||
- **What was done:**
|
||||
- **`lib/bds/preview_assets.ex`** — `generated_outputs/0`: Replaced `File.read!` with `File.read` inside `Enum.flat_map`, silently skipping files that become unreadable between `Path.wildcard` and read (TOCTOU race).
|
||||
- **`lib/bds/templates.ex`** — `upsert_template_from_file/3`: Replaced `File.read!` and pattern-matched `Frontmatter.parse_document` with a `with` chain returning `{:ok, template} | {:error, reason}`. Replaced `Repo.insert_or_update!` with `Repo.insert_or_update` to propagate changeset errors.
|
||||
- **`lib/bds/templates.ex`** — Updated all three callers: `rebuild_templates_from_files` logs a warning and skips bad files, `sync_template_from_file` and `import_orphan_template_file` map errors to `{:error, :not_found}`.
|
||||
- **`lib/bds/release_packaging.ex`** — Already fixed by CSM-030 (`File.write!` → `File.write`).
|
||||
- Added 8 tests in `test/bds/csm034_file_read_bang_test.exs`: source-level assertions for no bang file ops in all three files, functional tests for rebuild skipping bad templates, sync returning error on deleted file, import returning error on missing file, and preview_assets returning valid output tuples.
|
||||
|
||||
---
|
||||
|
||||
### CSM-035 — Process Dictionary (`Process.get/put`) Usage
|
||||
- **File:** `lib/bds/desktop/ui_locale.ex:32,49,65`
|
||||
- **What:** `UILocale.put/1` sets process dictionary (`Process.put(@key, locale)`) for UI locale. Used in `ShellLive.render` (Z. 550) and `MenuBar`.
|
||||
- **Fix:** This is isolated to the LiveView/MenuBar process so it's low-risk, but document the invariant explicitly: the process dict key `:bds_ui_locale` is set before each render call.
|
||||
### ~~CSM-035 — Process Dictionary (`Process.get/put`) Usage~~ ✅ FIXED
|
||||
- **Fixed:** 2026-05-27
|
||||
- **What was done:**
|
||||
- **`lib/bds/desktop/ui_locale.ex`** — Added explicit **Invariant** section to `@moduledoc` documenting that every code path evaluating HEEx templates with `translated/1,2` must call `UILocale.put/1` before template evaluation. Lists all three render boundaries: `ShellLive.render/1`, `SidebarComponents.sidebar_content/1`, and `MenuBar.mount/1` + `handle_info({:set_ui_locale, _})`.
|
||||
- Verified no raw `Process.put(:bds_ui_locale, ...)` or `Process.get(:bds_ui_locale)` exists outside `ui_locale.ex`.
|
||||
- Added 9 tests in `test/bds/csm035_process_dict_test.exs`: source-level assertions that no raw Process.put/get/delete for `:bds_ui_locale` exists outside the module, render boundary assertions that `ShellLive.render/1`, `sidebar_content/1`, and `MenuBar.mount/1` call `UILocale.put` before template evaluation, and functional tests for `put/1`, `current/0`, and `with_locale/2` (including nil-restore behavior).
|
||||
|
||||
---
|
||||
|
||||
### CSM-036 — Missing `@impl true` on GenServer Callbacks
|
||||
- **File:** `lib/bds/publishing.ex:46,61,71,75`
|
||||
- **What:** Only `init/1` (Z. 36) and the first `handle_call` (Z. 41) have `@impl true`. The remaining `handle_call` clauses at Z. 46, 61, 71, 75 lack it.
|
||||
- **Fix:** Add `@impl true` before every `handle_call`, `handle_cast`, `handle_info`, and `terminate`.
|
||||
### ~~CSM-036 — Missing `@impl true` on GenServer Callbacks~~ ✅ FIXED
|
||||
- **Fixed:** 2026-05-27
|
||||
- **What was done:**
|
||||
- Added `@impl true` before all four `handle_call` clauses that were missing it in `lib/bds/publishing.ex`: `{:update_job, ...}`, `{:should_upload_scp_file, ...}`, `{:mark_uploaded_scp_file, ...}`, and `{:upload_site, ...}`.
|
||||
- No `handle_cast`, `handle_info`, or `terminate` callbacks exist in this module; only `handle_call` needed fixing.
|
||||
- Added 2 tests in `test/bds/csm036_impl_true_test.exs`: source-level assertion that every `handle_call` clause is preceded by `@impl true`, and a guard test for any future `handle_cast`/`handle_info`/`terminate` callbacks.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -11,6 +11,21 @@ defmodule BDS.Desktop.UILocale do
|
||||
process dictionary directly. Use `with_locale/2` around any render or
|
||||
component that needs a locale binding; use `current/0` to read it.
|
||||
|
||||
## Invariant
|
||||
|
||||
Every code path that evaluates HEEx templates containing `translated/1,2`
|
||||
calls **must** call `UILocale.put/1` before template evaluation:
|
||||
|
||||
* `ShellLive.render/1` — sets locale at the top of every LiveView render.
|
||||
* `SidebarComponents.sidebar_content/1` — sets locale before the function
|
||||
component's HEEx (runs in the same process, may be called outside
|
||||
the parent render cycle via `send_update`).
|
||||
* `MenuBar.mount/1` and `MenuBar.handle_info({:set_ui_locale, _})` — set
|
||||
locale in the separate menu-bar process which has its own render cycle.
|
||||
|
||||
Violating this invariant causes `current/0` to return a stale or `nil`
|
||||
locale, producing untranslated UI text.
|
||||
|
||||
Direct use of `Process.put(:bds_ui_locale, _)` or
|
||||
`Process.get(:bds_ui_locale)` is forbidden outside this module.
|
||||
"""
|
||||
|
||||
@@ -15,6 +15,7 @@ defmodule BDS.Embeddings do
|
||||
|
||||
@duplicate_threshold 0.92
|
||||
@exact_match_score 0.999999
|
||||
@key_batch_size 199
|
||||
|
||||
def model_id, do: configured_backend().model_info().model_id
|
||||
def dimensions, do: configured_backend().model_info().dimensions
|
||||
@@ -73,7 +74,24 @@ defmodule BDS.Embeddings do
|
||||
order_by: [asc: post.created_at, asc: post.slug]
|
||||
)
|
||||
|
||||
Enum.each(posts, &sync_post_if_enabled(&1, refresh_index: false))
|
||||
existing_keys = preload_keys_by_post_id(project_id, Enum.map(posts, & &1.id))
|
||||
base_label = max_label_value()
|
||||
|
||||
{rows, _next_label} =
|
||||
Enum.reduce(posts, {[], base_label + 1}, fn post, {acc, next_label} ->
|
||||
existing_key = Map.get(existing_keys, post.id)
|
||||
|
||||
case compute_key_data(post, existing_key, next_label) do
|
||||
:skip ->
|
||||
{acc, next_label}
|
||||
|
||||
{:upsert, row} ->
|
||||
bump = if existing_key, do: 0, else: 1
|
||||
{[row | acc], next_label + bump}
|
||||
end
|
||||
end)
|
||||
|
||||
batch_upsert_keys(rows)
|
||||
:ok = rebuild_snapshot(project_id)
|
||||
{:ok, Enum.map(posts, & &1.id)}
|
||||
else
|
||||
@@ -104,12 +122,27 @@ defmodule BDS.Embeddings do
|
||||
where: key.project_id == ^project_id and key.post_id not in ^post_ids
|
||||
)
|
||||
|
||||
posts
|
||||
|> Enum.with_index(1)
|
||||
|> Enum.each(fn {post, index} ->
|
||||
sync_post_if_enabled(post, refresh_index: false)
|
||||
:ok = report_rebuild_progress(on_progress, index, total_posts, "embedding entries")
|
||||
end)
|
||||
existing_keys = preload_keys_by_post_id(project_id)
|
||||
base_label = max_label_value()
|
||||
|
||||
{rows, _next_label} =
|
||||
posts
|
||||
|> Enum.with_index(1)
|
||||
|> Enum.reduce({[], base_label + 1}, fn {post, index}, {acc, next_label} ->
|
||||
:ok = report_rebuild_progress(on_progress, index, total_posts, "embedding entries")
|
||||
existing_key = Map.get(existing_keys, post.id)
|
||||
|
||||
case compute_key_data(post, existing_key, next_label) do
|
||||
:skip ->
|
||||
{acc, next_label}
|
||||
|
||||
{:upsert, row} ->
|
||||
bump = if existing_key, do: 0, else: 1
|
||||
{[row | acc], next_label + bump}
|
||||
end
|
||||
end)
|
||||
|
||||
batch_upsert_keys(rows)
|
||||
|
||||
:ok = report_rebuild_phase(on_progress, 0.99, "Persisting embedding snapshot")
|
||||
:ok = rebuild_snapshot(project_id)
|
||||
@@ -196,6 +229,53 @@ defmodule BDS.Embeddings do
|
||||
end
|
||||
end
|
||||
|
||||
defp preload_keys_by_post_id(project_id) do
|
||||
Repo.all(from key in Key, where: key.project_id == ^project_id)
|
||||
|> Map.new(&{&1.post_id, &1})
|
||||
end
|
||||
|
||||
defp preload_keys_by_post_id(project_id, post_ids) do
|
||||
Repo.all(
|
||||
from key in Key,
|
||||
where: key.project_id == ^project_id and key.post_id in ^post_ids
|
||||
)
|
||||
|> Map.new(&{&1.post_id, &1})
|
||||
end
|
||||
|
||||
defp max_label_value do
|
||||
Repo.one(from key in Key, select: max(key.label)) || 0
|
||||
end
|
||||
|
||||
defp compute_key_data(%Post{} = post, existing_key, next_label) do
|
||||
body = resolve_post_body(post)
|
||||
raw_text = compose_embedding_source(post.title, body)
|
||||
content_hash = hash_text(raw_text)
|
||||
|
||||
if existing_key && existing_key.content_hash == content_hash do
|
||||
:skip
|
||||
else
|
||||
{:ok, vector} = embed_text(raw_text, post.language)
|
||||
label = if existing_key, do: existing_key.label, else: next_label
|
||||
{:upsert, [label, post.id, post.project_id, content_hash, Jason.encode!(vector)]}
|
||||
end
|
||||
end
|
||||
|
||||
defp batch_upsert_keys([]), do: :ok
|
||||
|
||||
defp batch_upsert_keys(rows) do
|
||||
rows
|
||||
|> Enum.chunk_every(@key_batch_size)
|
||||
|> Enum.each(fn chunk ->
|
||||
placeholders = Enum.map_join(chunk, ", ", fn _ -> "(?, ?, ?, ?, ?)" end)
|
||||
params = List.flatten(chunk)
|
||||
|
||||
Repo.query!(
|
||||
"INSERT INTO embedding_keys (label, post_id, project_id, content_hash, vector) VALUES #{placeholders} ON CONFLICT(label) DO UPDATE SET content_hash = excluded.content_hash, vector = excluded.vector",
|
||||
params
|
||||
)
|
||||
end)
|
||||
end
|
||||
|
||||
def remove_post(post_id) when is_binary(post_id) do
|
||||
project_id =
|
||||
case Repo.get_by(Key, post_id: post_id) do
|
||||
@@ -227,23 +307,24 @@ defmodule BDS.Embeddings do
|
||||
order_by: [asc: post.created_at, asc: post.slug]
|
||||
)
|
||||
|
||||
Enum.each(posts, fn post ->
|
||||
body = resolve_post_body(post)
|
||||
content_hash = hash_text(compose_embedding_source(post.title, body))
|
||||
existing_keys = preload_keys_by_post_id(project_id)
|
||||
base_label = max_label_value()
|
||||
|
||||
case Repo.get_by(Key, post_id: post.id, project_id: project_id) do
|
||||
%Key{content_hash: ^content_hash} ->
|
||||
:ok
|
||||
{rows, _next_label} =
|
||||
Enum.reduce(posts, {[], base_label + 1}, fn post, {acc, next_label} ->
|
||||
existing_key = Map.get(existing_keys, post.id)
|
||||
|
||||
_other ->
|
||||
:ok =
|
||||
sync_post_if_enabled(
|
||||
%{post | content: if(post.content in [nil, ""], do: body, else: post.content)},
|
||||
refresh_index: false
|
||||
)
|
||||
end
|
||||
end)
|
||||
case compute_key_data(post, existing_key, next_label) do
|
||||
:skip ->
|
||||
{acc, next_label}
|
||||
|
||||
{:upsert, row} ->
|
||||
bump = if existing_key, do: 0, else: 1
|
||||
{[row | acc], next_label + bump}
|
||||
end
|
||||
end)
|
||||
|
||||
batch_upsert_keys(rows)
|
||||
:ok = rebuild_snapshot(project_id)
|
||||
|
||||
indexed =
|
||||
|
||||
@@ -28,8 +28,11 @@ defmodule BDS.PreviewAssets do
|
||||
end)
|
||||
|> Enum.filter(&File.regular?/1)
|
||||
|> Enum.sort()
|
||||
|> Enum.map(fn path ->
|
||||
{Path.relative_to(path, @preview_root), File.read!(path)}
|
||||
|> Enum.flat_map(fn path ->
|
||||
case File.read(path) do
|
||||
{:ok, contents} -> [{Path.relative_to(path, @preview_root), contents}]
|
||||
{:error, _reason} -> []
|
||||
end
|
||||
end)
|
||||
end
|
||||
|
||||
|
||||
@@ -46,6 +46,7 @@ defmodule BDS.Publishing do
|
||||
{:reply, Repo.get(PublishJob, job_id), state}
|
||||
end
|
||||
|
||||
@impl true
|
||||
def handle_call({:update_job, job_id, attrs}, _from, state) do
|
||||
with %PublishJob{} = job <- Repo.get(PublishJob, job_id) do
|
||||
attrs = Map.put(attrs, :updated_at, Persistence.now_ms())
|
||||
@@ -55,6 +56,7 @@ defmodule BDS.Publishing do
|
||||
{:reply, :ok, state}
|
||||
end
|
||||
|
||||
@impl true
|
||||
def handle_call({:should_upload_scp_file, upload_key, local_mtime}, _from, state) do
|
||||
should_upload? =
|
||||
case state.scp_uploads[upload_key] do
|
||||
@@ -65,10 +67,12 @@ defmodule BDS.Publishing do
|
||||
{:reply, should_upload?, state}
|
||||
end
|
||||
|
||||
@impl true
|
||||
def handle_call({:mark_uploaded_scp_file, upload_key, local_mtime}, _from, state) do
|
||||
{:reply, :ok, put_in(state, [:scp_uploads, upload_key], local_mtime)}
|
||||
end
|
||||
|
||||
@impl true
|
||||
def handle_call({:upload_site, project_id, credentials, targets, opts}, _from, state) do
|
||||
job_id = "publish-" <> Integer.to_string(System.unique_integer([:positive, :monotonic]))
|
||||
uploader = build_uploader(Keyword.put_new(opts, :project_id, project_id))
|
||||
|
||||
@@ -4,6 +4,8 @@ defmodule BDS.Templates do
|
||||
including slug derivation, status transitions, and filesystem synchronization.
|
||||
"""
|
||||
|
||||
require Logger
|
||||
|
||||
import Ecto.Query
|
||||
import BDS.MapUtils, only: [attr: 2, maybe_put: 3]
|
||||
|
||||
@@ -184,10 +186,18 @@ defmodule BDS.Templates do
|
||||
templates =
|
||||
template_paths
|
||||
|> Enum.with_index(1)
|
||||
|> Enum.map(fn {path, index} ->
|
||||
template = upsert_template_from_file(project_id, project, path)
|
||||
|> Enum.flat_map(fn {path, index} ->
|
||||
result = upsert_template_from_file(project_id, project, path)
|
||||
:ok = report_rebuild_progress(on_progress, index, total_files, "template files")
|
||||
template
|
||||
|
||||
case result do
|
||||
{:ok, template} ->
|
||||
[template]
|
||||
|
||||
{:error, reason} ->
|
||||
Logger.warning("Skipping template #{path}: #{inspect(reason)}")
|
||||
[]
|
||||
end
|
||||
end)
|
||||
|
||||
remove_stale_published_templates(project_id, project, template_paths)
|
||||
@@ -241,10 +251,9 @@ defmodule BDS.Templates do
|
||||
project = Projects.get_project!(template.project_id)
|
||||
full_path = Path.join(Projects.project_data_dir(project), template.file_path)
|
||||
|
||||
if File.exists?(full_path) do
|
||||
{:ok, upsert_template_from_file(template.project_id, project, full_path)}
|
||||
else
|
||||
{:error, :not_found}
|
||||
case upsert_template_from_file(template.project_id, project, full_path) do
|
||||
{:ok, _template} = ok -> ok
|
||||
{:error, _reason} -> {:error, :not_found}
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -278,10 +287,9 @@ defmodule BDS.Templates do
|
||||
project = Projects.get_project!(project_id)
|
||||
full_path = Path.join(Projects.project_data_dir(project), relative_path)
|
||||
|
||||
if File.exists?(full_path) do
|
||||
{:ok, upsert_template_from_file(project_id, project, full_path)}
|
||||
else
|
||||
{:error, :not_found}
|
||||
case upsert_template_from_file(project_id, project, full_path) do
|
||||
{:ok, _template} = ok -> ok
|
||||
{:error, _reason} -> {:error, :not_found}
|
||||
end
|
||||
end
|
||||
|
||||
@@ -493,31 +501,33 @@ defmodule BDS.Templates do
|
||||
end
|
||||
|
||||
defp upsert_template_from_file(project_id, project, path) do
|
||||
contents = File.read!(path)
|
||||
{:ok, %{fields: fields}} = Frontmatter.parse_document(contents)
|
||||
relative_path = Path.relative_to(path, Projects.project_data_dir(project))
|
||||
now = Persistence.now_ms()
|
||||
|
||||
attrs = %{
|
||||
id: DocumentFields.get(fields, "id") || Ecto.UUID.generate(),
|
||||
project_id: project_id,
|
||||
slug: DocumentFields.fetch!(fields, "slug"),
|
||||
title: DocumentFields.get(fields, "title") || "",
|
||||
kind: parse_template_kind(DocumentFields.fetch!(fields, "kind")),
|
||||
enabled: Map.get(fields, "enabled", true),
|
||||
version: Map.get(fields, "version", 1),
|
||||
file_path: relative_path,
|
||||
status: :published,
|
||||
content: nil,
|
||||
created_at: DocumentFields.get(fields, "createdAt", now),
|
||||
updated_at: DocumentFields.get(fields, "updatedAt", now)
|
||||
}
|
||||
with {:ok, contents} <- File.read(path),
|
||||
{:ok, %{fields: fields}} <- Frontmatter.parse_document(contents) do
|
||||
now = Persistence.now_ms()
|
||||
|
||||
template = Repo.get_by(Template, project_id: project_id, slug: attrs.slug) || %Template{}
|
||||
attrs = %{
|
||||
id: DocumentFields.get(fields, "id") || Ecto.UUID.generate(),
|
||||
project_id: project_id,
|
||||
slug: DocumentFields.fetch!(fields, "slug"),
|
||||
title: DocumentFields.get(fields, "title") || "",
|
||||
kind: parse_template_kind(DocumentFields.fetch!(fields, "kind")),
|
||||
enabled: Map.get(fields, "enabled", true),
|
||||
version: Map.get(fields, "version", 1),
|
||||
file_path: relative_path,
|
||||
status: :published,
|
||||
content: nil,
|
||||
created_at: DocumentFields.get(fields, "createdAt", now),
|
||||
updated_at: DocumentFields.get(fields, "updatedAt", now)
|
||||
}
|
||||
|
||||
template
|
||||
|> Template.changeset(attrs)
|
||||
|> Repo.insert_or_update!()
|
||||
template = Repo.get_by(Template, project_id: project_id, slug: attrs.slug) || %Template{}
|
||||
|
||||
template
|
||||
|> Template.changeset(attrs)
|
||||
|> Repo.insert_or_update()
|
||||
end
|
||||
end
|
||||
|
||||
defp remove_stale_published_templates(project_id, project, template_paths) do
|
||||
|
||||
206
test/bds/csm033_batch_inserts_test.exs
Normal file
206
test/bds/csm033_batch_inserts_test.exs
Normal file
@@ -0,0 +1,206 @@
|
||||
defmodule BDS.CSM033BatchInsertsTest do
|
||||
use ExUnit.Case, async: false
|
||||
import Ecto.Query
|
||||
|
||||
describe "source-level: embeddings.ex uses batch inserts instead of Enum.each + individual writes" do
|
||||
setup do
|
||||
source = File.read!("lib/bds/embeddings.ex")
|
||||
%{source: source}
|
||||
end
|
||||
|
||||
test "no Enum.each calling sync_post_if_enabled in bulk paths", %{source: source} do
|
||||
refute source =~ "Enum.each(posts, &sync_post_if_enabled",
|
||||
"bulk paths should not use Enum.each with sync_post_if_enabled"
|
||||
|
||||
refute source =~ ~r/Enum\.each\(fn \{post, index\} ->\n\s+sync_post_if_enabled/,
|
||||
"bulk paths should not use Enum.each with sync_post_if_enabled"
|
||||
end
|
||||
|
||||
test "bulk functions use batch_upsert_keys", %{source: source} do
|
||||
assert source =~ "batch_upsert_keys(rows)",
|
||||
"expected batch_upsert_keys to be called with collected rows"
|
||||
end
|
||||
|
||||
test "bulk functions preload keys before the loop", %{source: source} do
|
||||
assert source =~ "preload_keys_by_post_id(project_id)",
|
||||
"expected keys to be preloaded in a single query"
|
||||
end
|
||||
|
||||
test "batch_upsert_keys uses multi-row INSERT with ON CONFLICT upsert", %{source: source} do
|
||||
assert source =~ "INSERT INTO embedding_keys",
|
||||
"expected raw SQL batch INSERT for embedding keys"
|
||||
|
||||
assert source =~ "ON CONFLICT(label) DO UPDATE",
|
||||
"expected ON CONFLICT upsert clause"
|
||||
end
|
||||
|
||||
test "compute_key_data is used instead of individual Repo.insert_or_update", %{source: source} do
|
||||
assert source =~ "compute_key_data(post, existing_key, next_label)",
|
||||
"expected compute_key_data helper for row computation"
|
||||
end
|
||||
end
|
||||
|
||||
describe "source-level: search.ex already uses batch inserts" do
|
||||
test "batch_insert_post_index uses multi-row VALUES" do
|
||||
source = File.read!("lib/bds/search.ex")
|
||||
assert source =~ "batch_insert_post_index"
|
||||
assert source =~ ~r/INSERT INTO posts_fts.*VALUES.*\#\{placeholders\}/s
|
||||
end
|
||||
|
||||
test "batch_insert_media_index uses multi-row VALUES" do
|
||||
source = File.read!("lib/bds/search.ex")
|
||||
assert source =~ "batch_insert_media_index"
|
||||
assert source =~ ~r/INSERT INTO media_fts.*VALUES.*\#\{placeholders\}/s
|
||||
end
|
||||
end
|
||||
|
||||
describe "functional: batch operations produce correct results" do
|
||||
defmodule FakeBackend do
|
||||
@behaviour BDS.Embeddings.Backend
|
||||
|
||||
@impl true
|
||||
def model_info, do: %{model_id: "fake/test-model", dimensions: 384}
|
||||
|
||||
@impl true
|
||||
def embed(text, opts), do: BDS.Embeddings.Backends.InApp.embed(text, opts)
|
||||
end
|
||||
|
||||
setup do
|
||||
:ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
|
||||
|
||||
temp_dir =
|
||||
Path.join(System.tmp_dir!(), "bds-csm033-#{System.unique_integer([:positive])}")
|
||||
|
||||
File.mkdir_p!(temp_dir)
|
||||
on_exit(fn -> File.rm_rf(temp_dir) end)
|
||||
|
||||
{:ok, project} = BDS.Projects.create_project(%{name: "CSM033", data_path: temp_dir})
|
||||
|
||||
previous_config = Application.get_env(:bds, :embeddings)
|
||||
Application.put_env(:bds, :embeddings, backend: FakeBackend)
|
||||
|
||||
on_exit(fn ->
|
||||
if previous_config == nil do
|
||||
Application.delete_env(:bds, :embeddings)
|
||||
else
|
||||
Application.put_env(:bds, :embeddings, previous_config)
|
||||
end
|
||||
end)
|
||||
|
||||
assert {:ok, _metadata} =
|
||||
BDS.Metadata.update_project_metadata(project.id, %{
|
||||
semantic_similarity_enabled: true
|
||||
})
|
||||
|
||||
%{project: project}
|
||||
end
|
||||
|
||||
test "index_unindexed batch-inserts keys for multiple posts", %{project: project} do
|
||||
posts =
|
||||
for i <- 1..5 do
|
||||
{:ok, post} =
|
||||
BDS.Posts.create_post(%{
|
||||
project_id: project.id,
|
||||
title: "Post #{i}",
|
||||
content: "content for post number #{i} with unique words #{:rand.uniform(10000)}",
|
||||
language: "en"
|
||||
})
|
||||
|
||||
post
|
||||
end
|
||||
|
||||
{:ok, indexed} = BDS.Embeddings.index_unindexed(project.id)
|
||||
assert length(indexed) == 5
|
||||
assert Enum.all?(posts, fn post -> post.id in indexed end)
|
||||
|
||||
keys =
|
||||
BDS.Repo.all(
|
||||
from(k in BDS.Embeddings.Key, where: k.project_id == ^project.id)
|
||||
)
|
||||
|
||||
assert length(keys) == 5
|
||||
labels = Enum.map(keys, & &1.label) |> Enum.sort()
|
||||
assert labels == Enum.to_list(1..5)
|
||||
end
|
||||
|
||||
test "rebuild_project updates stale keys via batch upsert", %{project: project} do
|
||||
{:ok, post} =
|
||||
BDS.Posts.create_post(%{
|
||||
project_id: project.id,
|
||||
title: "Rebuild Target",
|
||||
content: "original content for rebuild test",
|
||||
language: "en"
|
||||
})
|
||||
|
||||
{:ok, _indexed} = BDS.Embeddings.index_unindexed(project.id)
|
||||
|
||||
original_key =
|
||||
BDS.Repo.get_by!(BDS.Embeddings.Key, project_id: project.id, post_id: post.id)
|
||||
|
||||
{:ok, _post} = BDS.Posts.update_post(post.id, %{content: "completely different content now"})
|
||||
|
||||
{:ok, rebuilt_ids} = BDS.Embeddings.rebuild_project(project.id)
|
||||
assert post.id in rebuilt_ids
|
||||
|
||||
updated_key =
|
||||
BDS.Repo.get_by!(BDS.Embeddings.Key, project_id: project.id, post_id: post.id)
|
||||
|
||||
assert updated_key.label == original_key.label
|
||||
assert updated_key.content_hash != original_key.content_hash
|
||||
end
|
||||
|
||||
test "repair_posts batch-upserts for specified posts only", %{project: project} do
|
||||
{:ok, post_a} =
|
||||
BDS.Posts.create_post(%{
|
||||
project_id: project.id,
|
||||
title: "Repair A",
|
||||
content: "content A",
|
||||
language: "en"
|
||||
})
|
||||
|
||||
{:ok, _post_b} =
|
||||
BDS.Posts.create_post(%{
|
||||
project_id: project.id,
|
||||
title: "Repair B",
|
||||
content: "content B",
|
||||
language: "en"
|
||||
})
|
||||
|
||||
{:ok, _indexed} = BDS.Embeddings.index_unindexed(project.id)
|
||||
{:ok, repaired} = BDS.Embeddings.repair_posts(project.id, [post_a.id])
|
||||
assert repaired == [post_a.id]
|
||||
|
||||
keys =
|
||||
BDS.Repo.all(
|
||||
from(k in BDS.Embeddings.Key, where: k.project_id == ^project.id)
|
||||
)
|
||||
|
||||
assert length(keys) == 2
|
||||
end
|
||||
|
||||
test "index_unindexed skips posts with matching content hash", %{project: project} do
|
||||
{:ok, post} =
|
||||
BDS.Posts.create_post(%{
|
||||
project_id: project.id,
|
||||
title: "Skip Test",
|
||||
content: "unchanged content for skip test",
|
||||
language: "en"
|
||||
})
|
||||
|
||||
{:ok, _indexed} = BDS.Embeddings.index_unindexed(project.id)
|
||||
|
||||
key_before =
|
||||
BDS.Repo.get_by!(BDS.Embeddings.Key, project_id: project.id, post_id: post.id)
|
||||
|
||||
{:ok, _indexed} = BDS.Embeddings.index_unindexed(project.id)
|
||||
|
||||
key_after =
|
||||
BDS.Repo.get_by!(BDS.Embeddings.Key, project_id: project.id, post_id: post.id)
|
||||
|
||||
assert key_before.label == key_after.label
|
||||
assert key_before.content_hash == key_after.content_hash
|
||||
assert key_before.vector == key_after.vector
|
||||
end
|
||||
end
|
||||
|
||||
end
|
||||
107
test/bds/csm034_file_read_bang_test.exs
Normal file
107
test/bds/csm034_file_read_bang_test.exs
Normal file
@@ -0,0 +1,107 @@
|
||||
defmodule BDS.CSM034FileReadBangTest do
|
||||
use ExUnit.Case, async: false
|
||||
import Ecto.Query
|
||||
|
||||
describe "source-level: no File.read! or File.write! in affected files" do
|
||||
test "preview_assets.ex has no File.read!" do
|
||||
source = File.read!("lib/bds/preview_assets.ex")
|
||||
refute source =~ "File.read!", "preview_assets.ex should use File.read, not File.read!"
|
||||
end
|
||||
|
||||
test "templates.ex has no File.read!" do
|
||||
source = File.read!("lib/bds/templates.ex")
|
||||
refute source =~ "File.read!", "templates.ex should use File.read, not File.read!"
|
||||
end
|
||||
|
||||
test "templates.ex has no File.write!" do
|
||||
source = File.read!("lib/bds/templates.ex")
|
||||
refute source =~ "File.write!", "templates.ex should use File.write, not File.write!"
|
||||
end
|
||||
|
||||
test "release_packaging.ex has no File.read! or File.write!" do
|
||||
source = File.read!("lib/bds/release_packaging.ex")
|
||||
refute source =~ "File.read!", "release_packaging.ex should use File.read, not File.read!"
|
||||
refute source =~ "File.write!", "release_packaging.ex should use File.write, not File.write!"
|
||||
end
|
||||
end
|
||||
|
||||
describe "preview_assets.generated_outputs/0 handles read errors" do
|
||||
test "returns results for readable files, skips unreadable ones" do
|
||||
outputs = BDS.PreviewAssets.generated_outputs()
|
||||
assert is_list(outputs)
|
||||
assert Enum.all?(outputs, fn {path, body} -> is_binary(path) and is_binary(body) end)
|
||||
end
|
||||
end
|
||||
|
||||
describe "templates file error handling" do
|
||||
setup do
|
||||
:ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
|
||||
temp_dir = Path.join(System.tmp_dir!(), "bds-csm034-#{System.unique_integer([:positive])}")
|
||||
File.mkdir_p!(temp_dir)
|
||||
on_exit(fn -> File.rm_rf(temp_dir) end)
|
||||
|
||||
{:ok, project} = BDS.Projects.create_project(%{name: "CSM034", data_path: temp_dir})
|
||||
%{project: project, temp_dir: temp_dir}
|
||||
end
|
||||
|
||||
test "rebuild skips unreadable template files without crashing", %{
|
||||
project: project,
|
||||
temp_dir: temp_dir
|
||||
} do
|
||||
templates_dir = Path.join(temp_dir, "templates")
|
||||
File.mkdir_p!(templates_dir)
|
||||
|
||||
File.write!(Path.join(templates_dir, "good.liquid"), """
|
||||
---
|
||||
slug: good
|
||||
kind: post
|
||||
title: Good Template
|
||||
---
|
||||
<p>{{ content }}</p>
|
||||
""")
|
||||
|
||||
File.write!(Path.join(templates_dir, "bad.liquid"), "no frontmatter here")
|
||||
|
||||
assert {:ok, templates} = BDS.Templates.rebuild_templates_from_files(project.id)
|
||||
assert length(templates) == 1
|
||||
assert hd(templates).slug == "good"
|
||||
end
|
||||
|
||||
test "sync_template_from_file returns error when file is deleted", %{
|
||||
project: project,
|
||||
temp_dir: temp_dir
|
||||
} do
|
||||
templates_dir = Path.join(temp_dir, "templates")
|
||||
File.mkdir_p!(templates_dir)
|
||||
|
||||
path = Path.join(templates_dir, "ephemeral.liquid")
|
||||
|
||||
File.write!(path, """
|
||||
---
|
||||
slug: ephemeral
|
||||
kind: post
|
||||
title: Ephemeral
|
||||
---
|
||||
<p>{{ content }}</p>
|
||||
""")
|
||||
|
||||
{:ok, _templates} = BDS.Templates.rebuild_templates_from_files(project.id)
|
||||
|
||||
template =
|
||||
BDS.Repo.one(
|
||||
from(t in BDS.Templates.Template,
|
||||
where: t.project_id == ^project.id and t.slug == "ephemeral"
|
||||
)
|
||||
)
|
||||
|
||||
File.rm!(path)
|
||||
|
||||
assert {:error, :not_found} = BDS.Templates.sync_template_from_file(template.id)
|
||||
end
|
||||
|
||||
test "import_orphan_template_file returns error for missing file", %{project: project} do
|
||||
assert {:error, :not_found} =
|
||||
BDS.Templates.import_orphan_template_file(project.id, "templates/ghost.liquid")
|
||||
end
|
||||
end
|
||||
end
|
||||
86
test/bds/csm035_process_dict_test.exs
Normal file
86
test/bds/csm035_process_dict_test.exs
Normal file
@@ -0,0 +1,86 @@
|
||||
defmodule BDS.CSM035ProcessDictTest do
|
||||
use ExUnit.Case, async: true
|
||||
|
||||
alias BDS.Desktop.UILocale
|
||||
|
||||
describe "source-level: no raw Process.put/get for :bds_ui_locale outside ui_locale.ex" do
|
||||
test "no direct Process.put(:bds_ui_locale, ...) outside ui_locale.ex" do
|
||||
elixir_files =
|
||||
Path.wildcard("lib/**/*.ex")
|
||||
|> Enum.reject(&(&1 == "lib/bds/desktop/ui_locale.ex"))
|
||||
|
||||
violations =
|
||||
Enum.filter(elixir_files, fn path ->
|
||||
source = File.read!(path)
|
||||
source =~ "Process.put(:bds_ui_locale" or source =~ "Process.put(@key"
|
||||
end)
|
||||
|
||||
assert violations == [],
|
||||
"Raw Process.put(:bds_ui_locale) found outside ui_locale.ex: #{inspect(violations)}"
|
||||
end
|
||||
|
||||
test "no direct Process.get(:bds_ui_locale, ...) outside ui_locale.ex" do
|
||||
elixir_files =
|
||||
Path.wildcard("lib/**/*.ex")
|
||||
|> Enum.reject(&(&1 == "lib/bds/desktop/ui_locale.ex"))
|
||||
|
||||
violations =
|
||||
Enum.filter(elixir_files, fn path ->
|
||||
source = File.read!(path)
|
||||
source =~ "Process.get(:bds_ui_locale" or source =~ "Process.delete(:bds_ui_locale"
|
||||
end)
|
||||
|
||||
assert violations == [],
|
||||
"Raw Process.get/delete(:bds_ui_locale) found outside ui_locale.ex: #{inspect(violations)}"
|
||||
end
|
||||
end
|
||||
|
||||
describe "source-level: render boundaries call UILocale.put before template evaluation" do
|
||||
test "ShellLive.render/1 calls UILocale.put before index(assigns)" do
|
||||
source = File.read!("lib/bds/desktop/shell_live.ex")
|
||||
render_match = Regex.run(~r/def render\(assigns\).*?\n(.*?)\n(.*?)\n/s, source)
|
||||
assert render_match, "could not find render/1 in shell_live.ex"
|
||||
[_, first_line | _] = render_match
|
||||
assert first_line =~ "UILocale.put", "render/1 must call UILocale.put on its first line"
|
||||
end
|
||||
|
||||
test "sidebar_content/1 calls UILocale.put before HEEx" do
|
||||
source = File.read!("lib/bds/desktop/shell_live/sidebar_components.ex")
|
||||
match = Regex.run(~r/def sidebar_content\(assigns\).*?\n(.*?)\n/s, source)
|
||||
assert match, "could not find sidebar_content/1"
|
||||
[_, first_line | _] = match
|
||||
assert first_line =~ "UILocale.put", "sidebar_content/1 must call UILocale.put on its first line"
|
||||
end
|
||||
|
||||
test "MenuBar.mount/1 calls UILocale.put" do
|
||||
source = File.read!("lib/bds/desktop/menu_bar.ex")
|
||||
match = Regex.run(~r/def mount\(menu\).*?\n(.*?)\n/s, source)
|
||||
assert match, "could not find mount/1 in menu_bar.ex"
|
||||
[_, first_line | _] = match
|
||||
assert first_line =~ "UILocale.put", "MenuBar.mount/1 must call UILocale.put on its first line"
|
||||
end
|
||||
end
|
||||
|
||||
describe "UILocale functional behavior" do
|
||||
test "put/1 sets locale readable by current/0" do
|
||||
UILocale.put("de")
|
||||
assert UILocale.current() == "de"
|
||||
end
|
||||
|
||||
test "with_locale/2 restores previous locale after block" do
|
||||
UILocale.put("en")
|
||||
UILocale.with_locale("fr", fn -> assert UILocale.current() == "fr" end)
|
||||
assert UILocale.current() == "en"
|
||||
end
|
||||
|
||||
test "with_locale/2 restores nil when no prior locale was set" do
|
||||
Process.delete(:bds_ui_locale)
|
||||
UILocale.with_locale("it", fn -> assert UILocale.current() == "it" end)
|
||||
assert UILocale.current() == nil
|
||||
end
|
||||
|
||||
test "put/1 returns :ok" do
|
||||
assert UILocale.put("en") == :ok
|
||||
end
|
||||
end
|
||||
end
|
||||
45
test/bds/csm036_impl_true_test.exs
Normal file
45
test/bds/csm036_impl_true_test.exs
Normal file
@@ -0,0 +1,45 @@
|
||||
defmodule BDS.CSM036ImplTrueTest do
|
||||
use ExUnit.Case, async: true
|
||||
|
||||
@publishing_source File.read!("lib/bds/publishing.ex")
|
||||
|
||||
describe "CSM-036: @impl true on GenServer callbacks" do
|
||||
test "every handle_call clause has @impl true" do
|
||||
lines = String.split(@publishing_source, "\n")
|
||||
|
||||
handle_call_lines =
|
||||
lines
|
||||
|> Enum.with_index(1)
|
||||
|> Enum.filter(fn {line, _idx} ->
|
||||
String.contains?(line, "def handle_call(")
|
||||
end)
|
||||
|
||||
assert length(handle_call_lines) >= 5, "expected at least 5 handle_call clauses"
|
||||
|
||||
for {_line, idx} <- handle_call_lines do
|
||||
preceding = Enum.at(lines, idx - 2)
|
||||
|
||||
assert String.contains?(preceding, "@impl true"),
|
||||
"handle_call at line #{idx} missing @impl true (preceding line: #{inspect(preceding)})"
|
||||
end
|
||||
end
|
||||
|
||||
test "no handle_cast, handle_info, or terminate without @impl true" do
|
||||
lines = String.split(@publishing_source, "\n")
|
||||
|
||||
callback_lines =
|
||||
lines
|
||||
|> Enum.with_index(1)
|
||||
|> Enum.filter(fn {line, _idx} ->
|
||||
Regex.match?(~r/^\s+def (handle_cast|handle_info|terminate)\(/, line)
|
||||
end)
|
||||
|
||||
for {_line, idx} <- callback_lines do
|
||||
preceding = Enum.at(lines, idx - 2)
|
||||
|
||||
assert String.contains?(preceding, "@impl true"),
|
||||
"callback at line #{idx} missing @impl true"
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user