From 7e9cc72e1fcce5c0dc4bec37a4bbd113d4a3940c Mon Sep 17 00:00:00 2001 From: Chili Palmer Date: Sat, 30 May 2026 09:48:32 +0200 Subject: [PATCH] fix: D1-16 cancel orphaned debounce timer so index saves coalesce; add tests --- SPECGAPS.md | 2 +- lib/bds/embeddings/index.ex | 15 ++++++++ test/bds/embeddings_test.exs | 67 ++++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/SPECGAPS.md b/SPECGAPS.md index e72be42..5c8f921 100644 --- a/SPECGAPS.md +++ b/SPECGAPS.md @@ -130,7 +130,7 @@ All reconciled to follow code. Specs must be self-consistent and match code. | D1-13 | ~~DiscardPostChangesSideEffects~~ | engine_side_effects.allium:99-104 | **Resolved:** test added in `posts_test.exs` — a published post is dirtied with unsaved title/content edits (re-indexing the dirty text in FTS), then `discard_post_changes/1` restores the published file version (status=published, content=nil, original title) and re-syncs the FTS index so the published terms are searchable again and the discarded edits are gone | | D1-14 | ~~ReplaceMediaFileSideEffects~~ | engine_side_effects.allium:128-134 | **Resolved:** 3 tests added in `media_test.exs` — `replace_media_file/2` copies the new image over the existing path, updates the row (checksum/size/width/height), and regenerates all thumbnails synchronously (present immediately after the call, no `.bak` backup left); identical-checksum replace is a no-op (`{:ok, nil}`); unknown media id returns `{:error, :not_found}` | | D1-15 | ~~Drag-and-drop image chain~~ | action_patterns.allium:84-103 | **Resolved:** the chain had no handler — added `BDS.Desktop.ShellLive.EditorImageDrop` (`import_and_link/3` runs steps 1-4: import media + synchronous thumbnails + link to post + return `![](bds-media://id)` markdown; `enrich/3` runs background steps 5-6: AI analysis auto-applied with no modal + auto-translate cascade when `do_not_translate == false`). `PostEditor.handle_event("editor_image_dropped", ...)` runs the synchronous chain (works offline since import isn't AI), pushes the cursor insert, and spawns `enrich` only when airplane mode is off. MonacoEditor JS hook captures image drops on the editor surface and pushes the file path (`phx-target={@myself}` routes the hook event to the component); i18n for de/fr/it/es. 3 tests added (module chain incl. thumbnails+link+markdown, non-image link form, full LiveView drop in airplane mode asserting import/link/insert with no AI metadata). | -| D1-16 | DebouncedPersistence (5s) | embedding.allium:204-208 | Write test: index persistence debounced | +| D1-16 | ~~DebouncedPersistence (5s)~~ | embedding.allium:213-217 | **Resolved:** 3 tests added in `embeddings_test.exs` (DebouncedPersistence describe): `Index.put/3` schedules a per-project save `timer` with ~5s remaining (>4000ms, <=5000ms) instead of writing immediately (no `embeddings.usearch` on disk yet); rapid `put`s coalesce (each reschedules the single timer, the previous timer is cancelled so `Process.read_timer` returns false, and still no file after two writes); when the `{:save, project_id}` debounce message fires the index is persisted to disk and the pending `timer` cleared. The coalescing test exposed a real bug: `handle_call({:put, ...})` replaced the stored entry with `build_entry/2`'s fresh `timer: nil` entry before `schedule_save/2` ran, orphaning the previous debounce timer (left to fire a redundant save) instead of cancelling it; fixed via `cancel_pending_save/2` so bulk `put`s collapse to one deferred write | | D1-17 | Protected categories cannot be deleted | editor_settings.allium:81-84 | Write test: article/aside/page/picture deletion rejected | | D1-18 | HomeItemProtection (menu) | editor_misc.allium:206-209 | Write test: cannot move/reorder/delete Home | diff --git a/lib/bds/embeddings/index.ex b/lib/bds/embeddings/index.ex index 703cacd..fab9d76 100644 --- a/lib/bds/embeddings/index.ex +++ b/lib/bds/embeddings/index.ex @@ -103,6 +103,10 @@ defmodule BDS.Embeddings.Index do @impl true def handle_call({:put, project_id, dimensions, entries}, _from, state) do + # Cancel any pending debounce for this project first: build_entry/2 returns a + # fresh entry with timer: nil, so without this the previous timer would be + # orphaned (left to fire a redundant save) instead of coalescing. + state = cancel_pending_save(state, project_id) entry = build_entry(dimensions, entries) state = state |> Map.put(project_id, entry) |> schedule_save(project_id) {:reply, :ok, state} @@ -265,6 +269,17 @@ defmodule BDS.Embeddings.Index do Map.put(state, project_id, %{entry | timer: timer}) end + defp cancel_pending_save(state, project_id) do + case Map.get(state, project_id) do + %{timer: timer} = entry when is_reference(timer) -> + Process.cancel_timer(timer) + Map.put(state, project_id, %{entry | timer: nil}) + + _other -> + state + end + end + defp save_now(state, project_id) do case Map.get(state, project_id) do nil -> diff --git a/test/bds/embeddings_test.exs b/test/bds/embeddings_test.exs index f1b5ec0..9f555a9 100644 --- a/test/bds/embeddings_test.exs +++ b/test/bds/embeddings_test.exs @@ -624,4 +624,71 @@ defmodule BDS.EmbeddingsTest do # Queries stay safe. assert {:ok, []} = BDS.Embeddings.find_similar(post.id, 5) end + + # DebouncedPersistence invariant (embedding.allium:213-217): HNSW index + # persistence is debounced at 5s so bulk operations don't thrash the disk; + # rapid writes coalesce into one deferred save, force-saved via flush. + describe "DebouncedPersistence invariant" do + alias BDS.Embeddings.Index + + defp packed_vector(seed) do + for offset <- 1..384, into: <<>>, do: <<:math.sin(seed + offset)::float-32-little>> + end + + defp index_entries do + [ + %{label: 1, post_id: 101, vector: packed_vector(1)}, + %{label: 2, post_id: 102, vector: packed_vector(2)} + ] + end + + test "put schedules a ~5s save timer instead of writing to disk immediately", %{ + project: project + } do + refute File.exists?(Index.path(project.id)) + + :ok = Index.put(project.id, 384, index_entries()) + + entry = Map.fetch!(:sys.get_state(Index), project.id) + assert is_reference(entry.timer) + + # the debounce window is 5 seconds + remaining = Process.read_timer(entry.timer) + assert is_integer(remaining) + assert remaining > 4_000 and remaining <= 5_000 + + # nothing flushed to disk yet — persistence is deferred + refute File.exists?(Index.path(project.id)) + end + + test "rapid puts coalesce: each reschedules the single debounce timer", %{project: project} do + :ok = Index.put(project.id, 384, index_entries()) + first_timer = Map.fetch!(:sys.get_state(Index), project.id).timer + + :ok = Index.put(project.id, 384, index_entries()) + second_timer = Map.fetch!(:sys.get_state(Index), project.id).timer + + # a new timer replaced the old one (debounce reset) and the old one was + # cancelled, so two writes still produce only one pending save + refute first_timer == second_timer + assert Process.read_timer(first_timer) == false + + refute File.exists?(Index.path(project.id)) + end + + test "the debounce timer firing flushes to disk and clears the pending timer", %{ + project: project + } do + :ok = Index.put(project.id, 384, index_entries()) + refute File.exists?(Index.path(project.id)) + + # simulate the 5s debounce elapsing + send(Index, {:save, project.id}) + # synchronous round-trip ensures the {:save, _} message was processed first + entry = Map.fetch!(:sys.get_state(Index), project.id) + + assert File.exists?(Index.path(project.id)) + assert is_nil(entry.timer) + end + end end