From d8b24c9b7298545b0431470ab55c7ec48af3e519 Mon Sep 17 00:00:00 2001 From: Chili Palmer Date: Thu, 11 Jun 2026 21:34:26 +0200 Subject: [PATCH] fix: implemented TD-04, embedding indexes flush to disk on shutdown --- TECHDEBTS.md | 15 ++++++++++++++- lib/bds/desktop/shutdown.ex | 11 ++++++++++- test/bds/desktop_test.exs | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/TECHDEBTS.md b/TECHDEBTS.md index c8bfe0a..d0a6b47 100644 --- a/TECHDEBTS.md +++ b/TECHDEBTS.md @@ -168,10 +168,23 @@ concurrent-first-use race is impossible by construction. --- -### TD-04: Flush embedding indexes on shutdown (or delete the dead `flush_all`) +### TD-04: Flush embedding indexes on shutdown (or delete the dead `flush_all`) ✅ DONE (2026-06-11) **Severity: Medium (perf/contract), High confidence.** +**Status: implemented.** `Shutdown.persist_safely/0` now calls +`BDS.Embeddings.Index.flush_all()` next to `MainWindow.persist_now()`; each +persist step is hardened individually (own rescue/catch) so one failure never +blocks quit or skips the other step. `terminate/2` stays as defense-in-depth +for supervised restarts. A test proves a debounced (unsaved) index reaches +disk through the real shutdown path before the hard quit fires. The +`terminate/2` audit found no other graceful-shutdown dependency: +`job_runner.ex` only detaches in-memory state (moot under SIGKILL), +`automation.ex` is the test-automation harness whose ports die with the VM, +and `main_window.ex` bounds persistence was already covered by +`MainWindow.persist_now()` in the shutdown path. The code now matches the +spec's DebouncedPersistence invariant (`specs/embedding.allium:216`). + **Context.** App shutdown SIGKILLs the BEAM (`BDS.Desktop.Shutdown.quit/0` — a documented and legitimate workaround for a wxWidgets static-destructor segfault on macOS). Consequence: **no `terminate/2` callback in the whole app diff --git a/lib/bds/desktop/shutdown.ex b/lib/bds/desktop/shutdown.ex index c67199e..b77d1cd 100644 --- a/lib/bds/desktop/shutdown.ex +++ b/lib/bds/desktop/shutdown.ex @@ -89,8 +89,17 @@ defmodule BDS.Desktop.Shutdown do :ok end + # quit/0 SIGKILLs the BEAM, so no terminate/2 callback ever runs on shutdown; + # everything that must reach disk has to be flushed here. Each step is + # hardened individually so one failure never blocks quit or the other steps. defp persist_safely do - MainWindow.persist_now() + persist_step(fn -> MainWindow.persist_now() end) + persist_step(fn -> BDS.Embeddings.Index.flush_all() end) + :ok + end + + defp persist_step(fun) do + fun.() :ok rescue _error -> :ok diff --git a/test/bds/desktop_test.exs b/test/bds/desktop_test.exs index 1a03f3d..bed23e3 100644 --- a/test/bds/desktop_test.exs +++ b/test/bds/desktop_test.exs @@ -256,6 +256,39 @@ defmodule BDS.DesktopTest do assert_receive :window_quit_requested end + test "app-owned shutdown flushes pending embedding index saves before hard quit" do + previous_module = Application.get_env(:bds, :desktop_shutdown_module) + previous_quit_module = Application.get_env(:bds, :desktop_window_quit_module) + previous_pid = Application.get_env(:bds, :desktop_shutdown_test_pid) + + Application.put_env(:bds, :desktop_shutdown_module, BDS.Desktop.Shutdown) + Application.put_env(:bds, :desktop_window_quit_module, FakeWindowQuit) + Application.put_env(:bds, :desktop_shutdown_test_pid, self()) + + project_id = "shutdown-flush-#{System.unique_integer([:positive])}" + index_path = BDS.Embeddings.Index.path(project_id) + + on_exit(fn -> + restore_env(:desktop_shutdown_module, previous_module) + restore_env(:desktop_window_quit_module, previous_quit_module) + restore_env(:desktop_shutdown_test_pid, previous_pid) + :ok = BDS.Embeddings.Index.forget(project_id) + File.rm_rf(Path.dirname(index_path)) + end) + + vector = for offset <- 1..384, into: <<>>, do: <<:math.sin(offset)::float-32-little>> + :ok = BDS.Embeddings.Index.put(project_id, 384, [%{label: 1, post_id: 101, vector: vector}]) + + # the save is debounced; without a shutdown flush nothing is on disk yet + refute File.exists?(index_path) + + assert :ok = BDS.Desktop.Shutdown.request_quit() + # quit is the final shutdown step, so persistence has completed by now + assert_receive :window_quit_requested + + assert File.exists?(index_path) + end + test "the app owns final termination instead of delegating to Desktop.Window/System.halt" do # Desktop.Window.quit/0 routes through System.halt/1, which runs the wx C++ # static destructors on exit and crashes on macOS. The app-owned shutdown